| Message ID | 1431158038-3813-8-git-send-email-mcoquelin.stm32@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 09.05.2015 um 09:53 schrieb Maxime Coquelin: > The STM32 MCUs family IPs can be reset by accessing some registers > from the RCC block. > > The list of available reset lines is documented in the DT bindings. > > Tested-by: Chanwoo Choi <cw00.choi@samsung.com> > Acked-by: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com> > --- > drivers/reset/Makefile | 1 + > drivers/reset/reset-stm32.c | 124 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 125 insertions(+) > create mode 100644 drivers/reset/reset-stm32.c > > diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile > index 157d421..aed12d1 100644 > --- a/drivers/reset/Makefile > +++ b/drivers/reset/Makefile > @@ -1,5 +1,6 @@ > obj-$(CONFIG_RESET_CONTROLLER) += core.o > obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o > obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o > +obj-$(CONFIG_ARCH_STM32) += reset-stm32.o > obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o > obj-$(CONFIG_ARCH_STI) += sti/ > diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c > new file mode 100644 > index 0000000..2c41858 > --- /dev/null > +++ b/drivers/reset/reset-stm32.c [...] > +static const struct of_device_id stm32_reset_dt_ids[] = { > + { .compatible = "st,stm32-rcc", }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, sstm32_reset_dt_ids); Typo. IIUC the timer depends on the reset controller, so it must be built in anyway, and that's what's enforced in the Makefile above. Drop the line? Regards, Andreas
2015-05-21 1:45 GMT+02:00 Andreas Färber <afaerber@suse.de>: > Am 09.05.2015 um 09:53 schrieb Maxime Coquelin: >> +static const struct of_device_id stm32_reset_dt_ids[] = { >> + { .compatible = "st,stm32-rcc", }, >> + { /* sentinel */ }, >> +}; >> +MODULE_DEVICE_TABLE(of, sstm32_reset_dt_ids); > > Typo. Indeed, thanks for pointing this out. > > IIUC the timer depends on the reset controller, so it must be built in > anyway, and that's what's enforced in the Makefile above. Drop the line? > Agree it must be built-in. I will fix that in next version. Thanks for the review, Maxime
Am 21.05.2015 um 09:46 schrieb Maxime Coquelin: > 2015-05-21 1:45 GMT+02:00 Andreas Färber <afaerber@suse.de>: >> Am 09.05.2015 um 09:53 schrieb Maxime Coquelin: >>> +static const struct of_device_id stm32_reset_dt_ids[] = { >>> + { .compatible = "st,stm32-rcc", }, >>> + { /* sentinel */ }, >>> +}; >>> +MODULE_DEVICE_TABLE(of, sstm32_reset_dt_ids); >> >> Typo. > > Indeed, thanks for pointing this out. > >> >> IIUC the timer depends on the reset controller, so it must be built in >> anyway, and that's what's enforced in the Makefile above. Drop the line? >> > > Agree it must be built-in. > I will fix that in next version. Actually, I've updated a timer implementation of mine to invoke a reset controller similar to how you do in the STM32 clocksource patch, but no reset controller is getting returned. It seems to me, you are working around that by simply ignoring the error in the timer code and not doing a reset then, so the STM32 timer does in fact _not_ depend on the reset controller? What happened to your efforts of making the reset controller usable for the timer? In my case, my timer is originally in reset state and needs to be deasserted, so I can't just ignore it. Regards, Andreas
2015-05-21 19:58 GMT+02:00 Andreas Färber <afaerber@suse.de>: > Am 21.05.2015 um 09:46 schrieb Maxime Coquelin: >> 2015-05-21 1:45 GMT+02:00 Andreas Färber <afaerber@suse.de>: >>> Am 09.05.2015 um 09:53 schrieb Maxime Coquelin: >>>> +static const struct of_device_id stm32_reset_dt_ids[] = { >>>> + { .compatible = "st,stm32-rcc", }, >>>> + { /* sentinel */ }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, sstm32_reset_dt_ids); >>> >>> Typo. >> >> Indeed, thanks for pointing this out. >> >>> >>> IIUC the timer depends on the reset controller, so it must be built in >>> anyway, and that's what's enforced in the Makefile above. Drop the line? >>> >> >> Agree it must be built-in. >> I will fix that in next version. > > Actually, I've updated a timer implementation of mine to invoke a reset > controller similar to how you do in the STM32 clocksource patch, but no > reset controller is getting returned. > > It seems to me, you are working around that by simply ignoring the error > in the timer code and not doing a reset then, so the STM32 timer does in > fact _not_ depend on the reset controller? What happened to your efforts > of making the reset controller usable for the timer? In my case, my > timer is originally in reset state and needs to be deasserted, so I > can't just ignore it. Indeed, I made the reset optionnal in the clocksource patch since v3. Rob and Arnd said a lot of platform relies on such things are done by the bootloader [0]. I decided to deassert timers reset at bootloader stage, and make it optionnal in clocksource driver. I made it optionnal in case we decide one day to move reset initialization before timer are initialized. Note that for now, I still use your bootloader. I have done the changes to reset the timers in the afboot-stm32. That's the reason why I asked you under which licence it is delivered few months ago. I can share you the patch if you want, even if I understand it is more about the concept that you are reluctant. On my side, I plan to move to U-Boot soon, as Kamil Lulko added STM32 support in mainline [1]. In case of U-Boot, the timer reset should be de-asserted when jumping into the Kernel, as Rob mentionned [0]. Kind regards, Maxime [0]: https://lkml.org/lkml/2015/3/10/737 [1]: http://u-boot.10912.n7.nabble.com/PATCH-v2-stm32f4-fix-serial-output-td212812.html
Am 21.05.2015 um 21:57 schrieb Maxime Coquelin: > 2015-05-21 19:58 GMT+02:00 Andreas Färber <afaerber@suse.de>: >> Actually, I've updated a timer implementation of mine to invoke a reset >> controller similar to how you do in the STM32 clocksource patch, but no >> reset controller is getting returned. >> >> It seems to me, you are working around that by simply ignoring the error >> in the timer code and not doing a reset then, so the STM32 timer does in >> fact _not_ depend on the reset controller? What happened to your efforts >> of making the reset controller usable for the timer? In my case, my >> timer is originally in reset state and needs to be deasserted, so I >> can't just ignore it. > > Indeed, I made the reset optionnal in the clocksource patch since v3. > Rob and Arnd said a lot of platform relies on such things are done by > the bootloader [0]. > I decided to deassert timers reset at bootloader stage, and make it > optionnal in clocksource driver. > I made it optionnal in case we decide one day to move reset > initialization before timer are initialized. > > Note that for now, I still use your bootloader. > I have done the changes to reset the timers in the afboot-stm32. > That's the reason why I asked you under which licence it is delivered > few months ago. Sorry, too many mails... The stm32 one is GPL-2.0, as parts of it were derived from a U-Boot fork. (Personally I prefer GPL-2.0+; fm4 and xmc4000 are MIT/X11.) > I can share you the patch if you want, even if I understand it is more > about the concept that you are reluctant. > > On my side, I plan to move to U-Boot soon, as Kamil Lulko added STM32 > support in mainline [1]. You're free to use any bootloader you like, but you will find it difficult to build in USB etc. drivers given the sheer size of U-Boot. That was my motivation for writing the tiny one. ;) > In case of U-Boot, the timer reset should be de-asserted when jumping > into the Kernel, as Rob mentionned [0]. Thanks, I've updated the xmc4000 one accordingly and can do the same for stm32. But you are right that I consider that an ugly workaround, although on the other hand my earlyprintk patches also depend on the bootloader setting up GPIOs and UART. Regards, Andreas
2015-05-22 0:01 GMT+02:00 Andreas Färber <afaerber@suse.de>: > Am 21.05.2015 um 21:57 schrieb Maxime Coquelin: >> Note that for now, I still use your bootloader. >> I have done the changes to reset the timers in the afboot-stm32. >> That's the reason why I asked you under which licence it is delivered >> few months ago. > > Sorry, too many mails... The stm32 one is GPL-2.0, as parts of it were > derived from a U-Boot fork. (Personally I prefer GPL-2.0+; fm4 and > xmc4000 are MIT/X11.) Not a problem, thanks for providing the licence. >> I can share you the patch if you want, even if I understand it is more >> about the concept that you are reluctant. >> >> On my side, I plan to move to U-Boot soon, as Kamil Lulko added STM32 >> support in mainline [1]. > > You're free to use any bootloader you like, but you will find it > difficult to build in USB etc. drivers given the sheer size of U-Boot. > That was my motivation for writing the tiny one. ;) I think the two bootloaders make sense. Indeed, using U-Boot restricts the size of the Kernel. I also have the stm32429i-eval board, with 32MB NOR and 32MB SD-Ram. At least on this one I will use U-Boot, as tftp could be used to load Kernel since it has Ethernet port. >> In case of U-Boot, the timer reset should be de-asserted when jumping >> into the Kernel, as Rob mentionned [0]. > > Thanks, I've updated the xmc4000 one accordingly and can do the same for > stm32. But you are right that I consider that an ugly workaround, > although on the other hand my earlyprintk patches also depend on the > bootloader setting up GPIOs and UART. Yes, the Kernel always need to rely on the bootloader to provide a minimal setup (clock/ddr/muxing...). Regards, Maxime
diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile index 157d421..aed12d1 100644 --- a/drivers/reset/Makefile +++ b/drivers/reset/Makefile @@ -1,5 +1,6 @@ obj-$(CONFIG_RESET_CONTROLLER) += core.o obj-$(CONFIG_ARCH_SOCFPGA) += reset-socfpga.o obj-$(CONFIG_ARCH_BERLIN) += reset-berlin.o +obj-$(CONFIG_ARCH_STM32) += reset-stm32.o obj-$(CONFIG_ARCH_SUNXI) += reset-sunxi.o obj-$(CONFIG_ARCH_STI) += sti/ diff --git a/drivers/reset/reset-stm32.c b/drivers/reset/reset-stm32.c new file mode 100644 index 0000000..2c41858 --- /dev/null +++ b/drivers/reset/reset-stm32.c @@ -0,0 +1,124 @@ +/* + * Copyright (C) Maxime Coquelin 2015 + * Author: Maxime Coquelin <mcoquelin.stm32@gmail.com> + * License terms: GNU General Public License (GPL), version 2 + * + * Heavily based on sunxi driver from Maxime Ripard. + */ + +#include <linux/err.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/types.h> + +struct stm32_reset_data { + spinlock_t lock; + void __iomem *membase; + struct reset_controller_dev rcdev; +}; + +static int stm32_reset_assert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct stm32_reset_data *data = container_of(rcdev, + struct stm32_reset_data, + rcdev); + int bank = id / BITS_PER_LONG; + int offset = id % BITS_PER_LONG; + unsigned long flags; + u32 reg; + + spin_lock_irqsave(&data->lock, flags); + + reg = readl_relaxed(data->membase + (bank * 4)); + writel_relaxed(reg | BIT(offset), data->membase + (bank * 4)); + + spin_unlock_irqrestore(&data->lock, flags); + + return 0; +} + +static int stm32_reset_deassert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct stm32_reset_data *data = container_of(rcdev, + struct stm32_reset_data, + rcdev); + int bank = id / BITS_PER_LONG; + int offset = id % BITS_PER_LONG; + unsigned long flags; + u32 reg; + + spin_lock_irqsave(&data->lock, flags); + + reg = readl_relaxed(data->membase + (bank * 4)); + writel_relaxed(reg & ~BIT(offset), data->membase + (bank * 4)); + + spin_unlock_irqrestore(&data->lock, flags); + + return 0; +} + +static struct reset_control_ops stm32_reset_ops = { + .assert = stm32_reset_assert, + .deassert = stm32_reset_deassert, +}; + +static const struct of_device_id stm32_reset_dt_ids[] = { + { .compatible = "st,stm32-rcc", }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, sstm32_reset_dt_ids); + +static int stm32_reset_probe(struct platform_device *pdev) +{ + struct stm32_reset_data *data; + struct resource *res; + + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + data->membase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(data->membase)) + return PTR_ERR(data->membase); + + spin_lock_init(&data->lock); + + data->rcdev.owner = THIS_MODULE; + data->rcdev.nr_resets = resource_size(res) * 8; + data->rcdev.ops = &stm32_reset_ops; + data->rcdev.of_node = pdev->dev.of_node; + + return reset_controller_register(&data->rcdev); +} + +static int stm32_reset_remove(struct platform_device *pdev) +{ + struct stm32_reset_data *data = platform_get_drvdata(pdev); + + reset_controller_unregister(&data->rcdev); + + return 0; +} + +static struct platform_driver stm32_reset_driver = { + .probe = stm32_reset_probe, + .remove = stm32_reset_remove, + .driver = { + .name = "stm32-rcc-reset", + .of_match_table = stm32_reset_dt_ids, + }, +}; +module_platform_driver(stm32_reset_driver); + +MODULE_AUTHOR("Maxime Coquelin <maxime.coquelin@gmail.com>"); +MODULE_DESCRIPTION("STM32 MCUs Reset Controller Driver"); +MODULE_LICENSE("GPL");