Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-30 12:54, Arnd Bergmann wrote: > On Saturday 29 November 2014 01:15:57 Stefan Agner wrote: >> On 2014-11-28 23:22, Arnd Bergmann wrote: >> > On Friday 28 November 2014 23:09:09 Stefan Agner wrote: >> >> On 2014-11-28 22:24, Arnd Bergmann wrote: >> >> > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: >> >> > >> >> >> > If the SRC is also capable of resetting individual blocks instead of >> >> >> > just >> >> >> > the entire machine, it would be a reset driver in drivers/reset >> >> >> > instead. >> >> >> >> >> >> Beside the system reset, there is only a mask functionality for the >> >> >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the >> >> >> M4). This makes the SRC module in the Vybrid a bit different then what >> >> >> is available on other i.MX SoC's... >> >> > >> >> > If you already have the watchdog registers in there and want to have >> >> > a watchdog driver too, the easiest way would be to register the reboot >> >> > handler from the watchdog driver. >> >> >> >> Hm, not sure we speak about the same here. The SRC module has two >> >> (multi-)bit fields to mask the watchdog reset event for each watchdog. >> >> Beside that, there are two full watchdog register maps, which are in >> >> different areas. There is already a driver for this watchdogs. I'm not >> >> sure what the idea behind this is exactly, I guess it would easily allow >> >> to (temporary) mask the other CPU's watchdog. However, I don't think we >> >> need that functionality, so I don't care about that right now. >> > >> > Ok, I see, thanks for the clarification! >> > >> >> There is also a restart handler in the watchdog driver, but I prefer to >> >> use the reset capabilities of the SRC since it has immediate effect. >> >> >> >> Lets get to the big picture again: I could register the whole SRC >> >> register map as a syscon device and then access the registers from my >> >> suspend/resume implementation later on. And similar in the restart >> >> driver, I would use syscon_regmap_lookup_by_compatible to check if it >> >> contains the vf610-src compatible string and register the restart >> >> driver/handler if available. >> >> One thing which came into my mind regarding suspend: I might need to >> access the registers from assembler (in SRAM), can I do that through >> syscon/regmap? I had a quick look, but I don't found a way to get back >> the mapped IO base address.. By good reasons, of course, for most >> applications. But in my case, afaik I have no other choice. > > Yes, I can see that being a problem. What register specifically do > you need to access from code running in SRAM? > There are three registers I need in my current code, VF610_SRC_GPR0, VF610_SRC_GPR1 and VF610_SRC_MISC2. The last can done from C code, as it is only DDR RESET behavior during LP mode which need to be configured. GPR0 is the location to jump at on wakeup, and GPR1 the argument to it. The argument is the base address in SRAM, but the location, GPR0, is currently calculated in assembler. I guess I can calculate that in C code too. Currently I do not have a symbol which I can access, the whole suspend and resume is done in one "function", similar as it is done for i.MX6: arch/arm/mach-imx/suspend-imx6.S -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Saturday 29 November 2014 01:15:57 Stefan Agner wrote: > On 2014-11-28 23:22, Arnd Bergmann wrote: > > On Friday 28 November 2014 23:09:09 Stefan Agner wrote: > >> On 2014-11-28 22:24, Arnd Bergmann wrote: > >> > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: > >> > > >> >> > If the SRC is also capable of resetting individual blocks instead of > >> >> > just > >> >> > the entire machine, it would be a reset driver in drivers/reset > >> >> > instead. > >> >> > >> >> Beside the system reset, there is only a mask functionality for the > >> >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the > >> >> M4). This makes the SRC module in the Vybrid a bit different then what > >> >> is available on other i.MX SoC's... > >> > > >> > If you already have the watchdog registers in there and want to have > >> > a watchdog driver too, the easiest way would be to register the reboot > >> > handler from the watchdog driver. > >> > >> Hm, not sure we speak about the same here. The SRC module has two > >> (multi-)bit fields to mask the watchdog reset event for each watchdog. > >> Beside that, there are two full watchdog register maps, which are in > >> different areas. There is already a driver for this watchdogs. I'm not > >> sure what the idea behind this is exactly, I guess it would easily allow > >> to (temporary) mask the other CPU's watchdog. However, I don't think we > >> need that functionality, so I don't care about that right now. > > > > Ok, I see, thanks for the clarification! > > > >> There is also a restart handler in the watchdog driver, but I prefer to > >> use the reset capabilities of the SRC since it has immediate effect. > >> > >> Lets get to the big picture again: I could register the whole SRC > >> register map as a syscon device and then access the registers from my > >> suspend/resume implementation later on. And similar in the restart > >> driver, I would use syscon_regmap_lookup_by_compatible to check if it > >> contains the vf610-src compatible string and register the restart > >> driver/handler if available. > > One thing which came into my mind regarding suspend: I might need to > access the registers from assembler (in SRAM), can I do that through > syscon/regmap? I had a quick look, but I don't found a way to get back > the mapped IO base address.. By good reasons, of course, for most > applications. But in my case, afaik I have no other choice. Yes, I can see that being a problem. What register specifically do you need to access from code running in SRAM? Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Saturday 29 November 2014 01:15:57 Stefan Agner wrote: On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. One thing which came into my mind regarding suspend: I might need to access the registers from assembler (in SRAM), can I do that through syscon/regmap? I had a quick look, but I don't found a way to get back the mapped IO base address.. By good reasons, of course, for most applications. But in my case, afaik I have no other choice. Yes, I can see that being a problem. What register specifically do you need to access from code running in SRAM? Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-30 12:54, Arnd Bergmann wrote: On Saturday 29 November 2014 01:15:57 Stefan Agner wrote: On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. One thing which came into my mind regarding suspend: I might need to access the registers from assembler (in SRAM), can I do that through syscon/regmap? I had a quick look, but I don't found a way to get back the mapped IO base address.. By good reasons, of course, for most applications. But in my case, afaik I have no other choice. Yes, I can see that being a problem. What register specifically do you need to access from code running in SRAM? There are three registers I need in my current code, VF610_SRC_GPR0, VF610_SRC_GPR1 and VF610_SRC_MISC2. The last can done from C code, as it is only DDR RESET behavior during LP mode which need to be configured. GPR0 is the location to jump at on wakeup, and GPR1 the argument to it. The argument is the base address in SRAM, but the location, GPR0, is currently calculated in assembler. I guess I can calculate that in C code too. Currently I do not have a symbol which I can access, the whole suspend and resume is done in one function, similar as it is done for i.MX6: arch/arm/mach-imx/suspend-imx6.S -- Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 23:22, Arnd Bergmann wrote: > On Friday 28 November 2014 23:09:09 Stefan Agner wrote: >> On 2014-11-28 22:24, Arnd Bergmann wrote: >> > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: >> > >> >> > If the SRC is also capable of resetting individual blocks instead of >> >> > just >> >> > the entire machine, it would be a reset driver in drivers/reset instead. >> >> >> >> Beside the system reset, there is only a mask functionality for the >> >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the >> >> M4). This makes the SRC module in the Vybrid a bit different then what >> >> is available on other i.MX SoC's... >> > >> > If you already have the watchdog registers in there and want to have >> > a watchdog driver too, the easiest way would be to register the reboot >> > handler from the watchdog driver. >> >> Hm, not sure we speak about the same here. The SRC module has two >> (multi-)bit fields to mask the watchdog reset event for each watchdog. >> Beside that, there are two full watchdog register maps, which are in >> different areas. There is already a driver for this watchdogs. I'm not >> sure what the idea behind this is exactly, I guess it would easily allow >> to (temporary) mask the other CPU's watchdog. However, I don't think we >> need that functionality, so I don't care about that right now. > > Ok, I see, thanks for the clarification! > >> There is also a restart handler in the watchdog driver, but I prefer to >> use the reset capabilities of the SRC since it has immediate effect. >> >> Lets get to the big picture again: I could register the whole SRC >> register map as a syscon device and then access the registers from my >> suspend/resume implementation later on. And similar in the restart >> driver, I would use syscon_regmap_lookup_by_compatible to check if it >> contains the vf610-src compatible string and register the restart >> driver/handler if available. One thing which came into my mind regarding suspend: I might need to access the registers from assembler (in SRAM), can I do that through syscon/regmap? I had a quick look, but I don't found a way to get back the mapped IO base address.. By good reasons, of course, for most applications. But in my case, afaik I have no other choice. -- Stefan > > Correct, and also in the watchdog driver, I guess. > > Instead of syscon_regmap_lookup_by_compatible, please use > syscon_regmap_lookup_by_phandle and put the link to the syscon > device into the device accessing it. > > Also, see if you can use or extend drivers/power/reset/syscon-reboot.c > for your use case. > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 11/28/2014 03:00 PM, Stefan Agner wrote: On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Nice, this allows to do the reset with almost no code. The only thing which might be a problem is the priority: I used 192 since 128 is already used by the watchdog driver. Maybe we can change that syscon-reboot is a bit above watchdogs by default, or maybe even a dt property? I will try to use that driver for v2. Thx! A dt property might make more sense if the syscon driver is to be used for multiple systems. Otherwise, no matter what you pick, it will be wrong for some system. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 23:22, Arnd Bergmann wrote: > On Friday 28 November 2014 23:09:09 Stefan Agner wrote: >> On 2014-11-28 22:24, Arnd Bergmann wrote: >> > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: >> > >> >> > If the SRC is also capable of resetting individual blocks instead of >> >> > just >> >> > the entire machine, it would be a reset driver in drivers/reset instead. >> >> >> >> Beside the system reset, there is only a mask functionality for the >> >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the >> >> M4). This makes the SRC module in the Vybrid a bit different then what >> >> is available on other i.MX SoC's... >> > >> > If you already have the watchdog registers in there and want to have >> > a watchdog driver too, the easiest way would be to register the reboot >> > handler from the watchdog driver. >> >> Hm, not sure we speak about the same here. The SRC module has two >> (multi-)bit fields to mask the watchdog reset event for each watchdog. >> Beside that, there are two full watchdog register maps, which are in >> different areas. There is already a driver for this watchdogs. I'm not >> sure what the idea behind this is exactly, I guess it would easily allow >> to (temporary) mask the other CPU's watchdog. However, I don't think we >> need that functionality, so I don't care about that right now. > > Ok, I see, thanks for the clarification! > >> There is also a restart handler in the watchdog driver, but I prefer to >> use the reset capabilities of the SRC since it has immediate effect. >> >> Lets get to the big picture again: I could register the whole SRC >> register map as a syscon device and then access the registers from my >> suspend/resume implementation later on. And similar in the restart >> driver, I would use syscon_regmap_lookup_by_compatible to check if it >> contains the vf610-src compatible string and register the restart >> driver/handler if available. > > Correct, and also in the watchdog driver, I guess. > > Instead of syscon_regmap_lookup_by_compatible, please use > syscon_regmap_lookup_by_phandle and put the link to the syscon > device into the device accessing it. > > Also, see if you can use or extend drivers/power/reset/syscon-reboot.c > for your use case. Nice, this allows to do the reset with almost no code. The only thing which might be a problem is the priority: I used 192 since 128 is already used by the watchdog driver. Maybe we can change that syscon-reboot is a bit above watchdogs by default, or maybe even a dt property? I will try to use that driver for v2. Thx! -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 23:09:09 Stefan Agner wrote: > On 2014-11-28 22:24, Arnd Bergmann wrote: > > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: > > > >> > If the SRC is also capable of resetting individual blocks instead of just > >> > the entire machine, it would be a reset driver in drivers/reset instead. > >> > >> Beside the system reset, there is only a mask functionality for the > >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the > >> M4). This makes the SRC module in the Vybrid a bit different then what > >> is available on other i.MX SoC's... > > > > If you already have the watchdog registers in there and want to have > > a watchdog driver too, the easiest way would be to register the reboot > > handler from the watchdog driver. > > Hm, not sure we speak about the same here. The SRC module has two > (multi-)bit fields to mask the watchdog reset event for each watchdog. > Beside that, there are two full watchdog register maps, which are in > different areas. There is already a driver for this watchdogs. I'm not > sure what the idea behind this is exactly, I guess it would easily allow > to (temporary) mask the other CPU's watchdog. However, I don't think we > need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! > There is also a restart handler in the watchdog driver, but I prefer to > use the reset capabilities of the SRC since it has immediate effect. > > Lets get to the big picture again: I could register the whole SRC > register map as a syscon device and then access the registers from my > suspend/resume implementation later on. And similar in the restart > driver, I would use syscon_regmap_lookup_by_compatible to check if it > contains the vf610-src compatible string and register the restart > driver/handler if available. Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 22:24, Arnd Bergmann wrote: > On Friday 28 November 2014 22:02:01 Stefan Agner wrote: >> On 2014-11-28 17:49, Arnd Bergmann wrote: >> > On Friday 28 November 2014 17:43:35 Stefan Agner wrote: >> >> Support Vybrid SoC's system reset controller (SRC). Currently we >> >> don't register a reset controller but only support the imx_cpu_jump >> >> and imx_cpu_arg functions. >> >> >> >> Signed-off-by: Stefan Agner >> > >> > I think this should be a platform driver in drivers/power/reset. >> >> Yeah, I thought that too, see my cover letter. The problem is, in that >> module are also some register which are of interest when implementing >> suspend/resume support (see cover letter too). However, we could also >> just make a dt entry for that reset register only, and create another dt >> entry for the other registers. > > Don't make a node with just one register, in this case, a syscon device > would be best. Syscon seems like a match here. Was not aware of that, thx! > >> > If the SRC is also capable of resetting individual blocks instead of just >> > the entire machine, it would be a reset driver in drivers/reset instead. >> >> Beside the system reset, there is only a mask functionality for the >> watchdogs (there are two watchdogs, one for Cortex-A5 and one for the >> M4). This makes the SRC module in the Vybrid a bit different then what >> is available on other i.MX SoC's... > > If you already have the watchdog registers in there and want to have > a watchdog driver too, the easiest way would be to register the reboot > handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 22:02:01 Stefan Agner wrote: > On 2014-11-28 17:49, Arnd Bergmann wrote: > > On Friday 28 November 2014 17:43:35 Stefan Agner wrote: > >> Support Vybrid SoC's system reset controller (SRC). Currently we > >> don't register a reset controller but only support the imx_cpu_jump > >> and imx_cpu_arg functions. > >> > >> Signed-off-by: Stefan Agner > > > > I think this should be a platform driver in drivers/power/reset. > > Yeah, I thought that too, see my cover letter. The problem is, in that > module are also some register which are of interest when implementing > suspend/resume support (see cover letter too). However, we could also > just make a dt entry for that reset register only, and create another dt > entry for the other registers. Don't make a node with just one register, in this case, a syscon device would be best. > > If the SRC is also capable of resetting individual blocks instead of just > > the entire machine, it would be a reset driver in drivers/reset instead. > > Beside the system reset, there is only a mask functionality for the > watchdogs (there are two watchdogs, one for Cortex-A5 and one for the > M4). This makes the SRC module in the Vybrid a bit different then what > is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 17:49, Arnd Bergmann wrote: > On Friday 28 November 2014 17:43:35 Stefan Agner wrote: >> Support Vybrid SoC's system reset controller (SRC). Currently we >> don't register a reset controller but only support the imx_cpu_jump >> and imx_cpu_arg functions. >> >> Signed-off-by: Stefan Agner > > I think this should be a platform driver in drivers/power/reset. Yeah, I thought that too, see my cover letter. The problem is, in that module are also some register which are of interest when implementing suspend/resume support (see cover letter too). However, we could also just make a dt entry for that reset register only, and create another dt entry for the other registers. > If the SRC is also capable of resetting individual blocks instead of just > the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... -- Stefan > > Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 11/28/2014 08:43 AM, Stefan Agner wrote:
Support Vybrid SoC's system reset controller (SRC). Currently we
don't register a reset controller but only support the imx_cpu_jump
and imx_cpu_arg functions.
Signed-off-by: Stefan Agner
---
arch/arm/mach-imx/Makefile | 2 +-
arch/arm/mach-imx/common.h | 1 +
arch/arm/mach-imx/mach-vf610.c | 8 +++
arch/arm/mach-imx/src-vf610.c | 53 ++
4 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mach-imx/src-vf610.c
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index f5ac685..6f689fc 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -108,7 +108,7 @@ obj-$(CONFIG_SOC_IMX50) += mach-imx50.o
obj-$(CONFIG_SOC_IMX51) += mach-imx51.o
obj-$(CONFIG_SOC_IMX53) += mach-imx53.o
-obj-$(CONFIG_SOC_VF610) += clk-vf610.o mach-vf610.o
+obj-$(CONFIG_SOC_VF610) += clk-vf610.o src-vf610.o mach-vf610.o
obj-$(CONFIG_SOC_LS1021A) += mach-ls1021a.o
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 59ce8f3..458db03 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -102,6 +102,7 @@ static inline void imx_scu_map_io(void) {}
static inline void imx_smp_prepare(void) {}
#endif
void imx_src_init(void);
+void vf610_src_init(void);
void imx_gpc_init(void);
void imx_gpc_pre_suspend(bool arm_power_off);
void imx_gpc_post_resume(void);
diff --git a/arch/arm/mach-imx/mach-vf610.c b/arch/arm/mach-imx/mach-vf610.c
index c11ab6a..391c2b5 100644
--- a/arch/arm/mach-imx/mach-vf610.c
+++ b/arch/arm/mach-imx/mach-vf610.c
@@ -11,6 +11,13 @@
#include
#include
#include
+#include "common.h"
+
+static void __init vf610_init_machine(void)
+{
+ of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+ vf610_src_init();
+};
static const char * const vf610_dt_compat[] __initconst = {
"fsl,vf610",
@@ -20,5 +27,6 @@ static const char * const vf610_dt_compat[] __initconst = {
DT_MACHINE_START(VYBRID_VF610, "Freescale Vybrid VF610 (Device Tree)")
.l2c_aux_val= 0,
.l2c_aux_mask = ~0,
+ .init_machine = vf610_init_machine,
.dt_compat = vf610_dt_compat,
MACHINE_END
diff --git a/arch/arm/mach-imx/src-vf610.c b/arch/arm/mach-imx/src-vf610.c
new file mode 100644
index 000..5fba1d4
--- /dev/null
+++ b/arch/arm/mach-imx/src-vf610.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ * Copyright 2014 Toradex AG
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "common.h"
+
+#define SRC_SCR0x000
+#define SRC_GPR0 0x020
+#define BP_SRC_SCR_SW_RST 12
+
+static struct notifier_block restart_nb;
+static void __iomem *src_base;
+
+static int vf610_src_restart(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+ writel(1 << BP_SRC_SCR_SW_RST, src_base + SRC_SCR);
+ return NOTIFY_DONE;
+}
+
+void __init vf610_src_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "fsl,vf610-src");
+ if (!np)
+ return;
+
+ src_base = of_iomap(np, 0);
+ WARN_ON(!src_base);
It doesn't make much sense to register the restart handler if src_base is NULL.
+
+ restart_nb.notifier_call = vf610_src_restart;
+ restart_nb.priority = 192;
The above can be initialized statically in the restart_nb variable;
all you have to do is to move the variable below vf610_src_restart.
+ if (register_restart_handler(_nb))
+ printk(KERN_WARNING "failed to setup restart handler.\n");
I would suggest to use pr_warn().
Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 17:43:35 Stefan Agner wrote: > Support Vybrid SoC's system reset controller (SRC). Currently we > don't register a reset controller but only support the imx_cpu_jump > and imx_cpu_arg functions. > > Signed-off-by: Stefan Agner I think this should be a platform driver in drivers/power/reset. If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 17:43:35 Stefan Agner wrote: Support Vybrid SoC's system reset controller (SRC). Currently we don't register a reset controller but only support the imx_cpu_jump and imx_cpu_arg functions. Signed-off-by: Stefan Agner ste...@agner.ch I think this should be a platform driver in drivers/power/reset. If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 11/28/2014 08:43 AM, Stefan Agner wrote:
Support Vybrid SoC's system reset controller (SRC). Currently we
don't register a reset controller but only support the imx_cpu_jump
and imx_cpu_arg functions.
Signed-off-by: Stefan Agner ste...@agner.ch
---
arch/arm/mach-imx/Makefile | 2 +-
arch/arm/mach-imx/common.h | 1 +
arch/arm/mach-imx/mach-vf610.c | 8 +++
arch/arm/mach-imx/src-vf610.c | 53 ++
4 files changed, 63 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/mach-imx/src-vf610.c
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index f5ac685..6f689fc 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -108,7 +108,7 @@ obj-$(CONFIG_SOC_IMX50) += mach-imx50.o
obj-$(CONFIG_SOC_IMX51) += mach-imx51.o
obj-$(CONFIG_SOC_IMX53) += mach-imx53.o
-obj-$(CONFIG_SOC_VF610) += clk-vf610.o mach-vf610.o
+obj-$(CONFIG_SOC_VF610) += clk-vf610.o src-vf610.o mach-vf610.o
obj-$(CONFIG_SOC_LS1021A) += mach-ls1021a.o
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index 59ce8f3..458db03 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -102,6 +102,7 @@ static inline void imx_scu_map_io(void) {}
static inline void imx_smp_prepare(void) {}
#endif
void imx_src_init(void);
+void vf610_src_init(void);
void imx_gpc_init(void);
void imx_gpc_pre_suspend(bool arm_power_off);
void imx_gpc_post_resume(void);
diff --git a/arch/arm/mach-imx/mach-vf610.c b/arch/arm/mach-imx/mach-vf610.c
index c11ab6a..391c2b5 100644
--- a/arch/arm/mach-imx/mach-vf610.c
+++ b/arch/arm/mach-imx/mach-vf610.c
@@ -11,6 +11,13 @@
#include linux/irqchip.h
#include asm/mach/arch.h
#include asm/hardware/cache-l2x0.h
+#include common.h
+
+static void __init vf610_init_machine(void)
+{
+ of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+ vf610_src_init();
+};
static const char * const vf610_dt_compat[] __initconst = {
fsl,vf610,
@@ -20,5 +27,6 @@ static const char * const vf610_dt_compat[] __initconst = {
DT_MACHINE_START(VYBRID_VF610, Freescale Vybrid VF610 (Device Tree))
.l2c_aux_val= 0,
.l2c_aux_mask = ~0,
+ .init_machine = vf610_init_machine,
.dt_compat = vf610_dt_compat,
MACHINE_END
diff --git a/arch/arm/mach-imx/src-vf610.c b/arch/arm/mach-imx/src-vf610.c
new file mode 100644
index 000..5fba1d4
--- /dev/null
+++ b/arch/arm/mach-imx/src-vf610.c
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ * Copyright 2014 Toradex AG
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include linux/init.h
+#include linux/io.h
+#include linux/of.h
+#include linux/of_address.h
+#include linux/reboot.h
+#include linux/reset-controller.h
+#include linux/smp.h
+#include asm/smp_plat.h
+#include common.h
+
+#define SRC_SCR0x000
+#define SRC_GPR0 0x020
+#define BP_SRC_SCR_SW_RST 12
+
+static struct notifier_block restart_nb;
+static void __iomem *src_base;
+
+static int vf610_src_restart(struct notifier_block *nb, unsigned long action,
+ void *data)
+{
+ writel(1 BP_SRC_SCR_SW_RST, src_base + SRC_SCR);
+ return NOTIFY_DONE;
+}
+
+void __init vf610_src_init(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, fsl,vf610-src);
+ if (!np)
+ return;
+
+ src_base = of_iomap(np, 0);
+ WARN_ON(!src_base);
It doesn't make much sense to register the restart handler if src_base is NULL.
+
+ restart_nb.notifier_call = vf610_src_restart;
+ restart_nb.priority = 192;
The above can be initialized statically in the restart_nb variable;
all you have to do is to move the variable below vf610_src_restart.
+ if (register_restart_handler(restart_nb))
+ printk(KERN_WARNING failed to setup restart handler.\n);
I would suggest to use pr_warn().
Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 17:49, Arnd Bergmann wrote: On Friday 28 November 2014 17:43:35 Stefan Agner wrote: Support Vybrid SoC's system reset controller (SRC). Currently we don't register a reset controller but only support the imx_cpu_jump and imx_cpu_arg functions. Signed-off-by: Stefan Agner ste...@agner.ch I think this should be a platform driver in drivers/power/reset. Yeah, I thought that too, see my cover letter. The problem is, in that module are also some register which are of interest when implementing suspend/resume support (see cover letter too). However, we could also just make a dt entry for that reset register only, and create another dt entry for the other registers. If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... -- Stefan Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 22:02:01 Stefan Agner wrote: On 2014-11-28 17:49, Arnd Bergmann wrote: On Friday 28 November 2014 17:43:35 Stefan Agner wrote: Support Vybrid SoC's system reset controller (SRC). Currently we don't register a reset controller but only support the imx_cpu_jump and imx_cpu_arg functions. Signed-off-by: Stefan Agner ste...@agner.ch I think this should be a platform driver in drivers/power/reset. Yeah, I thought that too, see my cover letter. The problem is, in that module are also some register which are of interest when implementing suspend/resume support (see cover letter too). However, we could also just make a dt entry for that reset register only, and create another dt entry for the other registers. Don't make a node with just one register, in this case, a syscon device would be best. If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: On 2014-11-28 17:49, Arnd Bergmann wrote: On Friday 28 November 2014 17:43:35 Stefan Agner wrote: Support Vybrid SoC's system reset controller (SRC). Currently we don't register a reset controller but only support the imx_cpu_jump and imx_cpu_arg functions. Signed-off-by: Stefan Agner ste...@agner.ch I think this should be a platform driver in drivers/power/reset. Yeah, I thought that too, see my cover letter. The problem is, in that module are also some register which are of interest when implementing suspend/resume support (see cover letter too). However, we could also just make a dt entry for that reset register only, and create another dt entry for the other registers. Don't make a node with just one register, in this case, a syscon device would be best. Syscon seems like a match here. Was not aware of that, thx! If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. -- Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Nice, this allows to do the reset with almost no code. The only thing which might be a problem is the priority: I used 192 since 128 is already used by the watchdog driver. Maybe we can change that syscon-reboot is a bit above watchdogs by default, or maybe even a dt property? I will try to use that driver for v2. Thx! -- Stefan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 11/28/2014 03:00 PM, Stefan Agner wrote: On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Nice, this allows to do the reset with almost no code. The only thing which might be a problem is the priority: I used 192 since 128 is already used by the watchdog driver. Maybe we can change that syscon-reboot is a bit above watchdogs by default, or maybe even a dt property? I will try to use that driver for v2. Thx! A dt property might make more sense if the syscon driver is to be used for multiple systems. Otherwise, no matter what you pick, it will be wrong for some system. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] ARM: imx: src: support vf610 system reset controller
On 2014-11-28 23:22, Arnd Bergmann wrote: On Friday 28 November 2014 23:09:09 Stefan Agner wrote: On 2014-11-28 22:24, Arnd Bergmann wrote: On Friday 28 November 2014 22:02:01 Stefan Agner wrote: If the SRC is also capable of resetting individual blocks instead of just the entire machine, it would be a reset driver in drivers/reset instead. Beside the system reset, there is only a mask functionality for the watchdogs (there are two watchdogs, one for Cortex-A5 and one for the M4). This makes the SRC module in the Vybrid a bit different then what is available on other i.MX SoC's... If you already have the watchdog registers in there and want to have a watchdog driver too, the easiest way would be to register the reboot handler from the watchdog driver. Hm, not sure we speak about the same here. The SRC module has two (multi-)bit fields to mask the watchdog reset event for each watchdog. Beside that, there are two full watchdog register maps, which are in different areas. There is already a driver for this watchdogs. I'm not sure what the idea behind this is exactly, I guess it would easily allow to (temporary) mask the other CPU's watchdog. However, I don't think we need that functionality, so I don't care about that right now. Ok, I see, thanks for the clarification! There is also a restart handler in the watchdog driver, but I prefer to use the reset capabilities of the SRC since it has immediate effect. Lets get to the big picture again: I could register the whole SRC register map as a syscon device and then access the registers from my suspend/resume implementation later on. And similar in the restart driver, I would use syscon_regmap_lookup_by_compatible to check if it contains the vf610-src compatible string and register the restart driver/handler if available. One thing which came into my mind regarding suspend: I might need to access the registers from assembler (in SRAM), can I do that through syscon/regmap? I had a quick look, but I don't found a way to get back the mapped IO base address.. By good reasons, of course, for most applications. But in my case, afaik I have no other choice. -- Stefan Correct, and also in the watchdog driver, I guess. Instead of syscon_regmap_lookup_by_compatible, please use syscon_regmap_lookup_by_phandle and put the link to the syscon device into the device accessing it. Also, see if you can use or extend drivers/power/reset/syscon-reboot.c for your use case. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
