| Message ID | 1461567336-21834-1-git-send-email-andy.yan@rock-chips.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 04/25/2016 08:55 AM, Andy Yan wrote: > This driver parses the reboot commands like "reboot bootloader" > and "reboot recovery" to get a boot mode described in the > device tree , then call the write interfae to store the boot > mode in some place like special register or sram, which can > be read by the bootloader after system reboot, then the bootloader > can take different action according to the mode stored. > > This is commonly used on Android based devices, in order to > reboot the device into fastboot or recovery mode. > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> > Tested-by: John Stultz <john.stultz@linaro.org> > Acked-by: John Stultz <john.stultz@linaro.org> > Signed-off-by: Andy Yan <andy.yan@rock-chips.com> > > --- > > Changes in v8: > - do some cleanup when driver ubind > > Changes in v7: > - address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained. > > Changes in v6: None > Changes in v5: > - use two blank space under help in Kconfig > - use unsigned int instead of int for member magic in struct mode_info > > Changes in v4: > - make this driver depends on OF to avoid kbuild test error > > Changes in v3: > - scan multi properities > - add mask value for some platform which only use some bits of the register > to store boot mode magic value > > Changes in v2: > - move to dir drivers/power/reset/ > - make syscon-reboot-mode a generic driver > > Changes in v1: > - fix the embarrassed compile warning > - correct the maskrom magic number > - check for the normal reboot > > drivers/power/reset/Kconfig | 13 ++++ > drivers/power/reset/Makefile | 2 + > drivers/power/reset/reboot-mode.c | 118 +++++++++++++++++++++++++++++++ > drivers/power/reset/reboot-mode.h | 14 ++++ > drivers/power/reset/syscon-reboot-mode.c | 99 ++++++++++++++++++++++++++ > 5 files changed, 246 insertions(+) > create mode 100644 drivers/power/reset/reboot-mode.c > create mode 100644 drivers/power/reset/reboot-mode.h > create mode 100644 drivers/power/reset/syscon-reboot-mode.c > > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index 1131cf7..cf50630 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -173,5 +173,18 @@ config POWER_RESET_ZX > help > Reboot support for ZTE SoCs. > > +config REBOOT_MODE > + tristate > + > +config SYSCON_REBOOT_MODE > + bool "Generic SYSCON regmap reboot mode driver" > + depends on OF > + select REBOOT_MODE > + help > + Say y here will enable reboot mode driver. This will > + get reboot mode arguments and store it in SYSCON mapped > + register, then the bootloader can read it to take different > + action according to the mode. > + > endif > > diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile > index 096fa67..a63865b 100644 > --- a/drivers/power/reset/Makefile > +++ b/drivers/power/reset/Makefile > @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o > obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o > obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o > obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o > +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o > +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o > diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c > new file mode 100644 > index 0000000..cdc4d72 > --- /dev/null > +++ b/drivers/power/reset/reboot-mode.c > @@ -0,0 +1,118 @@ > +/* > + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#include <linux/device.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/reboot.h> > +#include "reboot-mode.h" > + > +#define PREFIX "mode-" > + > +struct mode_info { > + const char *mode; > + unsigned int magic; > + struct list_head list; > +}; > + > +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, > + const char *cmd) > +{ > + const char *normal = "normal"; > + int magic = 0; > + struct mode_info *info; > + > + if (!cmd) > + cmd = normal; > + > + list_for_each_entry(info, &reboot->head, list) { > + if (!strcmp(info->mode, cmd)) { > + magic = info->magic; > + break; > + } > + } > + > + return magic; > +} > + > +static int reboot_mode_notify(struct notifier_block *this, > + unsigned long mode, void *cmd) > +{ > + struct reboot_mode_driver *reboot; > + int magic; > + > + reboot = container_of(this, struct reboot_mode_driver, reboot_notifier); > + magic = get_reboot_mode_magic(reboot, cmd); > + if (magic) > + reboot->write(reboot, magic); > + > + return NOTIFY_DONE; > +} > + > +int reboot_mode_register(struct reboot_mode_driver *reboot) > +{ > + struct mode_info *info; > + struct property *prop; > + struct device_node *np = reboot->dev->of_node; > + size_t len = strlen(PREFIX); > + int ret; > + > + INIT_LIST_HEAD(&reboot->head); > + > + for_each_property_of_node(np, prop) { > + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len)) > + continue; > + > + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); > + if (!info) { > + ret = -ENOMEM; > + goto error; > + } > + > + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); > + if (of_property_read_u32(np, prop->name, &info->magic)) { > + dev_err(reboot->dev, "reboot mode %s without magic number\n", > + info->mode); > + devm_kfree(reboot->dev, info); kfree_const(). The duplicated string won't be added to the list so it won't be freed in error path or in unregister(). > + continue; > + } > + list_add_tail(&info->list, &reboot->head); > + } > + > + reboot->reboot_notifier.notifier_call = reboot_mode_notify; > + ret = register_reboot_notifier(&reboot->reboot_notifier); > + if (ret) > + dev_err(reboot->dev, "can't register reboot notifier\n"); I don't understand your error paths. In previous patches they were buggy... they still look buggy, I think. It's 8th iteration and such basic things are still present. If this is an error then shouldn't everything be cleaned up? You are returning error code thus the reboot_mode_register() caller will fail. This means the probe will fail. So you need to clean up - go to error? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof: On 2016?04?25? 18:42, Krzysztof Kozlowski wrote: > On 04/25/2016 08:55 AM, Andy Yan wrote: >> This driver parses the reboot commands like "reboot bootloader" >> and "reboot recovery" to get a boot mode described in the >> device tree , then call the write interfae to store the boot >> mode in some place like special register or sram, which can >> be read by the bootloader after system reboot, then the bootloader >> can take different action according to the mode stored. >> >> This is commonly used on Android based devices, in order to >> reboot the device into fastboot or recovery mode. >> >> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >> Tested-by: John Stultz <john.stultz@linaro.org> >> Acked-by: John Stultz <john.stultz@linaro.org> >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >> >> --- >> >> Changes in v8: >> - do some cleanup when driver ubind >> >> Changes in v7: >> - address some suggestions from Krzysztof, make syscon-reboot-mode driver data self-contained. >> >> Changes in v6: None >> Changes in v5: >> - use two blank space under help in Kconfig >> - use unsigned int instead of int for member magic in struct mode_info >> >> Changes in v4: >> - make this driver depends on OF to avoid kbuild test error >> >> Changes in v3: >> - scan multi properities >> - add mask value for some platform which only use some bits of the register >> to store boot mode magic value >> >> Changes in v2: >> - move to dir drivers/power/reset/ >> - make syscon-reboot-mode a generic driver >> >> Changes in v1: >> - fix the embarrassed compile warning >> - correct the maskrom magic number >> - check for the normal reboot >> >> drivers/power/reset/Kconfig | 13 ++++ >> drivers/power/reset/Makefile | 2 + >> drivers/power/reset/reboot-mode.c | 118 +++++++++++++++++++++++++++++++ >> drivers/power/reset/reboot-mode.h | 14 ++++ >> drivers/power/reset/syscon-reboot-mode.c | 99 ++++++++++++++++++++++++++ >> 5 files changed, 246 insertions(+) >> create mode 100644 drivers/power/reset/reboot-mode.c >> create mode 100644 drivers/power/reset/reboot-mode.h >> create mode 100644 drivers/power/reset/syscon-reboot-mode.c >> >> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig >> index 1131cf7..cf50630 100644 >> --- a/drivers/power/reset/Kconfig >> +++ b/drivers/power/reset/Kconfig >> @@ -173,5 +173,18 @@ config POWER_RESET_ZX >> help >> Reboot support for ZTE SoCs. >> >> +config REBOOT_MODE >> + tristate >> + >> +config SYSCON_REBOOT_MODE >> + bool "Generic SYSCON regmap reboot mode driver" >> + depends on OF >> + select REBOOT_MODE >> + help >> + Say y here will enable reboot mode driver. This will >> + get reboot mode arguments and store it in SYSCON mapped >> + register, then the bootloader can read it to take different >> + action according to the mode. >> + >> endif >> >> diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile >> index 096fa67..a63865b 100644 >> --- a/drivers/power/reset/Makefile >> +++ b/drivers/power/reset/Makefile >> @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o >> obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o >> obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o >> obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o >> +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o >> +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o >> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c >> new file mode 100644 >> index 0000000..cdc4d72 >> --- /dev/null >> +++ b/drivers/power/reset/reboot-mode.c >> @@ -0,0 +1,118 @@ >> +/* >> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/init.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/reboot.h> >> +#include "reboot-mode.h" >> + >> +#define PREFIX "mode-" >> + >> +struct mode_info { >> + const char *mode; >> + unsigned int magic; >> + struct list_head list; >> +}; >> + >> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, >> + const char *cmd) >> +{ >> + const char *normal = "normal"; >> + int magic = 0; >> + struct mode_info *info; >> + >> + if (!cmd) >> + cmd = normal; >> + >> + list_for_each_entry(info, &reboot->head, list) { >> + if (!strcmp(info->mode, cmd)) { >> + magic = info->magic; >> + break; >> + } >> + } >> + >> + return magic; >> +} >> + >> +static int reboot_mode_notify(struct notifier_block *this, >> + unsigned long mode, void *cmd) >> +{ >> + struct reboot_mode_driver *reboot; >> + int magic; >> + >> + reboot = container_of(this, struct reboot_mode_driver, reboot_notifier); >> + magic = get_reboot_mode_magic(reboot, cmd); >> + if (magic) >> + reboot->write(reboot, magic); >> + >> + return NOTIFY_DONE; >> +} >> + >> +int reboot_mode_register(struct reboot_mode_driver *reboot) >> +{ >> + struct mode_info *info; >> + struct property *prop; >> + struct device_node *np = reboot->dev->of_node; >> + size_t len = strlen(PREFIX); >> + int ret; >> + >> + INIT_LIST_HEAD(&reboot->head); >> + >> + for_each_property_of_node(np, prop) { >> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len)) >> + continue; >> + >> + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); >> + if (!info) { >> + ret = -ENOMEM; >> + goto error; >> + } >> + >> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); >> + if (of_property_read_u32(np, prop->name, &info->magic)) { >> + dev_err(reboot->dev, "reboot mode %s without magic number\n", >> + info->mode); >> + devm_kfree(reboot->dev, info); > kfree_const(). The duplicated string won't be added to the list so it > won't be freed in error path or in unregister(). Each mode has a mode_info, if one of them gets an error whith devm_kzalloc, they other mode_info add to the list before should be freed. > >> + continue; >> + } >> + list_add_tail(&info->list, &reboot->head); >> + } >> + >> + reboot->reboot_notifier.notifier_call = reboot_mode_notify; >> + ret = register_reboot_notifier(&reboot->reboot_notifier); >> + if (ret) >> + dev_err(reboot->dev, "can't register reboot notifier\n"); > I don't understand your error paths. In previous patches they were > buggy... they still look buggy, I think. It's 8th iteration and such > basic things are still present. > > If this is an error then shouldn't everything be cleaned up? You are > returning error code thus the reboot_mode_register() caller will fail. > This means the probe will fail. So you need to clean up - go to error? If this is an error, I think only the duplicated string should be clean up,because other resources allocated by devm. If there is something else, please let me know. thanks. > Best regards, > Krzysztof > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/25/2016 01:29 PM, Andy Yan wrote: > Hi Krzysztof: > > On 2016?04?25? 18:42, Krzysztof Kozlowski wrote: >> On 04/25/2016 08:55 AM, Andy Yan wrote: >>> This driver parses the reboot commands like "reboot bootloader" >>> and "reboot recovery" to get a boot mode described in the >>> device tree , then call the write interfae to store the boot >>> mode in some place like special register or sram, which can >>> be read by the bootloader after system reboot, then the bootloader >>> can take different action according to the mode stored. >>> >>> This is commonly used on Android based devices, in order to >>> reboot the device into fastboot or recovery mode. >>> >>> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >>> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >>> Tested-by: John Stultz <john.stultz@linaro.org> >>> Acked-by: John Stultz <john.stultz@linaro.org> >>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >>> >>> --- >>> >>> Changes in v8: >>> - do some cleanup when driver ubind >>> >>> Changes in v7: >>> - address some suggestions from Krzysztof, make syscon-reboot-mode >>> driver data self-contained. >>> >>> Changes in v6: None >>> Changes in v5: >>> - use two blank space under help in Kconfig >>> - use unsigned int instead of int for member magic in struct mode_info >>> >>> Changes in v4: >>> - make this driver depends on OF to avoid kbuild test error >>> >>> Changes in v3: >>> - scan multi properities >>> - add mask value for some platform which only use some bits of the >>> register >>> to store boot mode magic value >>> >>> Changes in v2: >>> - move to dir drivers/power/reset/ >>> - make syscon-reboot-mode a generic driver >>> >>> Changes in v1: >>> - fix the embarrassed compile warning >>> - correct the maskrom magic number >>> - check for the normal reboot >>> >>> drivers/power/reset/Kconfig | 13 ++++ >>> drivers/power/reset/Makefile | 2 + >>> drivers/power/reset/reboot-mode.c | 118 >>> +++++++++++++++++++++++++++++++ >>> drivers/power/reset/reboot-mode.h | 14 ++++ >>> drivers/power/reset/syscon-reboot-mode.c | 99 >>> ++++++++++++++++++++++++++ >>> 5 files changed, 246 insertions(+) >>> create mode 100644 drivers/power/reset/reboot-mode.c >>> create mode 100644 drivers/power/reset/reboot-mode.h >>> create mode 100644 drivers/power/reset/syscon-reboot-mode.c >>> >>> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig >>> index 1131cf7..cf50630 100644 >>> --- a/drivers/power/reset/Kconfig >>> +++ b/drivers/power/reset/Kconfig >>> @@ -173,5 +173,18 @@ config POWER_RESET_ZX >>> help >>> Reboot support for ZTE SoCs. >>> +config REBOOT_MODE >>> + tristate >>> + >>> +config SYSCON_REBOOT_MODE >>> + bool "Generic SYSCON regmap reboot mode driver" >>> + depends on OF >>> + select REBOOT_MODE >>> + help >>> + Say y here will enable reboot mode driver. This will >>> + get reboot mode arguments and store it in SYSCON mapped >>> + register, then the bootloader can read it to take different >>> + action according to the mode. >>> + >>> endif >>> diff --git a/drivers/power/reset/Makefile >>> b/drivers/power/reset/Makefile >>> index 096fa67..a63865b 100644 >>> --- a/drivers/power/reset/Makefile >>> +++ b/drivers/power/reset/Makefile >>> @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o >>> obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o >>> obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o >>> obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o >>> +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o >>> +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o >>> diff --git a/drivers/power/reset/reboot-mode.c >>> b/drivers/power/reset/reboot-mode.c >>> new file mode 100644 >>> index 0000000..cdc4d72 >>> --- /dev/null >>> +++ b/drivers/power/reset/reboot-mode.c >>> @@ -0,0 +1,118 @@ >>> +/* >>> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License as published by >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + */ >>> + >>> +#include <linux/device.h> >>> +#include <linux/init.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/of.h> >>> +#include <linux/reboot.h> >>> +#include "reboot-mode.h" >>> + >>> +#define PREFIX "mode-" >>> + >>> +struct mode_info { >>> + const char *mode; >>> + unsigned int magic; >>> + struct list_head list; >>> +}; >>> + >>> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, >>> + const char *cmd) >>> +{ >>> + const char *normal = "normal"; >>> + int magic = 0; >>> + struct mode_info *info; >>> + >>> + if (!cmd) >>> + cmd = normal; >>> + >>> + list_for_each_entry(info, &reboot->head, list) { >>> + if (!strcmp(info->mode, cmd)) { >>> + magic = info->magic; >>> + break; >>> + } >>> + } >>> + >>> + return magic; >>> +} >>> + >>> +static int reboot_mode_notify(struct notifier_block *this, >>> + unsigned long mode, void *cmd) >>> +{ >>> + struct reboot_mode_driver *reboot; >>> + int magic; >>> + >>> + reboot = container_of(this, struct reboot_mode_driver, >>> reboot_notifier); >>> + magic = get_reboot_mode_magic(reboot, cmd); >>> + if (magic) >>> + reboot->write(reboot, magic); >>> + >>> + return NOTIFY_DONE; >>> +} >>> + >>> +int reboot_mode_register(struct reboot_mode_driver *reboot) >>> +{ >>> + struct mode_info *info; >>> + struct property *prop; >>> + struct device_node *np = reboot->dev->of_node; >>> + size_t len = strlen(PREFIX); >>> + int ret; >>> + >>> + INIT_LIST_HEAD(&reboot->head); >>> + >>> + for_each_property_of_node(np, prop) { >>> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, >>> len)) >>> + continue; >>> + >>> + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); >>> + if (!info) { >>> + ret = -ENOMEM; >>> + goto error; >>> + } >>> + >>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); >>> + if (of_property_read_u32(np, prop->name, &info->magic)) { >>> + dev_err(reboot->dev, "reboot mode %s without magic >>> number\n", >>> + info->mode); >>> + devm_kfree(reboot->dev, info); >> kfree_const(). The duplicated string won't be added to the list so it >> won't be freed in error path or in unregister(). > > Each mode has a mode_info, if one of them gets an error whith > devm_kzalloc, they other mode_info add to the list before should be freed. Please, write it again. I don't understand. You are allocating 'info'. Then you are const-allocating 'info->mode'. In this error path you are freeing 'info'. Where is the kfree_const() for 'info->mode'? >> >>> + continue; >>> + } >>> + list_add_tail(&info->list, &reboot->head); >>> + } >>> + >>> + reboot->reboot_notifier.notifier_call = reboot_mode_notify; >>> + ret = register_reboot_notifier(&reboot->reboot_notifier); >>> + if (ret) >>> + dev_err(reboot->dev, "can't register reboot notifier\n"); >> I don't understand your error paths. In previous patches they were >> buggy... they still look buggy, I think. It's 8th iteration and such >> basic things are still present. >> >> If this is an error then shouldn't everything be cleaned up? You are >> returning error code thus the reboot_mode_register() caller will fail. >> This means the probe will fail. So you need to clean up - go to error? > > If this is an error, I think only the duplicated string should be > clean up,because other resources allocated by devm. If there is > something else, please let me know. Just goto error... Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof: On 2016?04?25? 20:48, Krzysztof Kozlowski wrote: > On 04/25/2016 01:29 PM, Andy Yan wrote: >> Hi Krzysztof: >> >> On 2016?04?25? 18:42, Krzysztof Kozlowski wrote: >>> On 04/25/2016 08:55 AM, Andy Yan wrote: >>>> This driver parses the reboot commands like "reboot bootloader" >>>> and "reboot recovery" to get a boot mode described in the >>>> device tree , then call the write interfae to store the boot >>>> mode in some place like special register or sram, which can >>>> be read by the bootloader after system reboot, then the bootloader >>>> can take different action according to the mode stored. >>>> >>>> This is commonly used on Android based devices, in order to >>>> reboot the device into fastboot or recovery mode. >>>> >>>> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >>>> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >>>> Tested-by: John Stultz <john.stultz@linaro.org> >>>> Acked-by: John Stultz <john.stultz@linaro.org> >>>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >>>> >>>> --- >>>> >>>> Changes in v8: >>>> - do some cleanup when driver ubind >>>> >>>> Changes in v7: >>>> - address some suggestions from Krzysztof, make syscon-reboot-mode >>>> driver data self-contained. >>>> >>>> Changes in v6: None >>>> Changes in v5: >>>> - use two blank space under help in Kconfig >>>> - use unsigned int instead of int for member magic in struct mode_info >>>> >>>> Changes in v4: >>>> - make this driver depends on OF to avoid kbuild test error >>>> >>>> Changes in v3: >>>> - scan multi properities >>>> - add mask value for some platform which only use some bits of the >>>> register >>>> to store boot mode magic value >>>> >>>> Changes in v2: >>>> - move to dir drivers/power/reset/ >>>> - make syscon-reboot-mode a generic driver >>>> >>>> Changes in v1: >>>> - fix the embarrassed compile warning >>>> - correct the maskrom magic number >>>> - check for the normal reboot >>>> >>>> drivers/power/reset/Kconfig | 13 ++++ >>>> drivers/power/reset/Makefile | 2 + >>>> drivers/power/reset/reboot-mode.c | 118 >>>> +++++++++++++++++++++++++++++++ >>>> drivers/power/reset/reboot-mode.h | 14 ++++ >>>> drivers/power/reset/syscon-reboot-mode.c | 99 >>>> ++++++++++++++++++++++++++ >>>> 5 files changed, 246 insertions(+) >>>> create mode 100644 drivers/power/reset/reboot-mode.c >>>> create mode 100644 drivers/power/reset/reboot-mode.h >>>> create mode 100644 drivers/power/reset/syscon-reboot-mode.c >>>> >>>> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig >>>> index 1131cf7..cf50630 100644 >>>> --- a/drivers/power/reset/Kconfig >>>> +++ b/drivers/power/reset/Kconfig >>>> @@ -173,5 +173,18 @@ config POWER_RESET_ZX >>>> help >>>> Reboot support for ZTE SoCs. >>>> +config REBOOT_MODE >>>> + tristate >>>> + >>>> +config SYSCON_REBOOT_MODE >>>> + bool "Generic SYSCON regmap reboot mode driver" >>>> + depends on OF >>>> + select REBOOT_MODE >>>> + help >>>> + Say y here will enable reboot mode driver. This will >>>> + get reboot mode arguments and store it in SYSCON mapped >>>> + register, then the bootloader can read it to take different >>>> + action according to the mode. >>>> + >>>> endif >>>> diff --git a/drivers/power/reset/Makefile >>>> b/drivers/power/reset/Makefile >>>> index 096fa67..a63865b 100644 >>>> --- a/drivers/power/reset/Makefile >>>> +++ b/drivers/power/reset/Makefile >>>> @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o >>>> obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o >>>> obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o >>>> obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o >>>> +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o >>>> +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o >>>> diff --git a/drivers/power/reset/reboot-mode.c >>>> b/drivers/power/reset/reboot-mode.c >>>> new file mode 100644 >>>> index 0000000..cdc4d72 >>>> --- /dev/null >>>> +++ b/drivers/power/reset/reboot-mode.c >>>> @@ -0,0 +1,118 @@ >>>> +/* >>>> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License as published by >>>> + * the Free Software Foundation; either version 2 of the License, or >>>> + * (at your option) any later version. >>>> + */ >>>> + >>>> +#include <linux/device.h> >>>> +#include <linux/init.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of.h> >>>> +#include <linux/reboot.h> >>>> +#include "reboot-mode.h" >>>> + >>>> +#define PREFIX "mode-" >>>> + >>>> +struct mode_info { >>>> + const char *mode; >>>> + unsigned int magic; >>>> + struct list_head list; >>>> +}; >>>> + >>>> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, >>>> + const char *cmd) >>>> +{ >>>> + const char *normal = "normal"; >>>> + int magic = 0; >>>> + struct mode_info *info; >>>> + >>>> + if (!cmd) >>>> + cmd = normal; >>>> + >>>> + list_for_each_entry(info, &reboot->head, list) { >>>> + if (!strcmp(info->mode, cmd)) { >>>> + magic = info->magic; >>>> + break; >>>> + } >>>> + } >>>> + >>>> + return magic; >>>> +} >>>> + >>>> +static int reboot_mode_notify(struct notifier_block *this, >>>> + unsigned long mode, void *cmd) >>>> +{ >>>> + struct reboot_mode_driver *reboot; >>>> + int magic; >>>> + >>>> + reboot = container_of(this, struct reboot_mode_driver, >>>> reboot_notifier); >>>> + magic = get_reboot_mode_magic(reboot, cmd); >>>> + if (magic) >>>> + reboot->write(reboot, magic); >>>> + >>>> + return NOTIFY_DONE; >>>> +} >>>> + >>>> +int reboot_mode_register(struct reboot_mode_driver *reboot) >>>> +{ >>>> + struct mode_info *info; >>>> + struct property *prop; >>>> + struct device_node *np = reboot->dev->of_node; >>>> + size_t len = strlen(PREFIX); >>>> + int ret; >>>> + >>>> + INIT_LIST_HEAD(&reboot->head); >>>> + >>>> + for_each_property_of_node(np, prop) { >>>> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, >>>> len)) >>>> + continue; >>>> + >>>> + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); >>>> + if (!info) { >>>> + ret = -ENOMEM; >>>> + goto error; >>>> + } >>>> + >>>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); >>>> + if (of_property_read_u32(np, prop->name, &info->magic)) { >>>> + dev_err(reboot->dev, "reboot mode %s without magic >>>> number\n", >>>> + info->mode); >>>> + devm_kfree(reboot->dev, info); >>> kfree_const(). The duplicated string won't be added to the list so it >>> won't be freed in error path or in unregister(). >> Each mode has a mode_info, if one of them gets an error whith >> devm_kzalloc, they other mode_info add to the list before should be freed. > Please, write it again. I don't understand. > > You are allocating 'info'. Then you are const-allocating 'info->mode'. > In this error path you are freeing 'info'. Where is the kfree_const() > for 'info->mode'? > Sorry, I got it, it's my fault.Thanks for your review. >>>> + continue; >>>> + } >>>> + list_add_tail(&info->list, &reboot->head); >>>> + } >>>> + >>>> + reboot->reboot_notifier.notifier_call = reboot_mode_notify; >>>> + ret = register_reboot_notifier(&reboot->reboot_notifier); >>>> + if (ret) >>>> + dev_err(reboot->dev, "can't register reboot notifier\n"); >>> I don't understand your error paths. In previous patches they were >>> buggy... they still look buggy, I think. It's 8th iteration and such >>> basic things are still present. >>> >>> If this is an error then shouldn't everything be cleaned up? You are >>> returning error code thus the reboot_mode_register() caller will fail. >>> This means the probe will fail. So you need to clean up - go to error? >> If this is an error, I think only the duplicated string should be >> clean up,because other resources allocated by devm. If there is >> something else, please let me know. > Just goto error... > > Best regards, > Krzysztof > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof: On 2016?04?25? 20:48, Krzysztof Kozlowski wrote: > On 04/25/2016 01:29 PM, Andy Yan wrote: >> Hi Krzysztof: >> >> On 2016?04?25? 18:42, Krzysztof Kozlowski wrote: >>> On 04/25/2016 08:55 AM, Andy Yan wrote: >>>> This driver parses the reboot commands like "reboot bootloader" >>>> and "reboot recovery" to get a boot mode described in the >>>> device tree , then call the write interfae to store the boot >>>> mode in some place like special register or sram, which can >>>> be read by the bootloader after system reboot, then the bootloader >>>> can take different action according to the mode stored. >>>> >>>> This is commonly used on Android based devices, in order to >>>> reboot the device into fastboot or recovery mode. >>>> >>>> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >>>> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >>>> Tested-by: John Stultz <john.stultz@linaro.org> >>>> Acked-by: John Stultz <john.stultz@linaro.org> >>>> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> >>>> >>>> --- >>>> >>>> Changes in v8: >>>> - do some cleanup when driver ubind >>>> >>>> Changes in v7: >>>> - address some suggestions from Krzysztof, make syscon-reboot-mode >>>> driver data self-contained. >>>> >>>> Changes in v6: None >>>> Changes in v5: >>>> - use two blank space under help in Kconfig >>>> - use unsigned int instead of int for member magic in struct mode_info >>>> >>>> Changes in v4: >>>> - make this driver depends on OF to avoid kbuild test error >>>> >>>> Changes in v3: >>>> - scan multi properities >>>> - add mask value for some platform which only use some bits of the >>>> register >>>> to store boot mode magic value >>>> >>>> Changes in v2: >>>> - move to dir drivers/power/reset/ >>>> - make syscon-reboot-mode a generic driver >>>> >>>> Changes in v1: >>>> - fix the embarrassed compile warning >>>> - correct the maskrom magic number >>>> - check for the normal reboot >>>> >>>> drivers/power/reset/Kconfig | 13 ++++ >>>> drivers/power/reset/Makefile | 2 + >>>> drivers/power/reset/reboot-mode.c | 118 >>>> +++++++++++++++++++++++++++++++ >>>> drivers/power/reset/reboot-mode.h | 14 ++++ >>>> drivers/power/reset/syscon-reboot-mode.c | 99 >>>> ++++++++++++++++++++++++++ >>>> 5 files changed, 246 insertions(+) >>>> create mode 100644 drivers/power/reset/reboot-mode.c >>>> create mode 100644 drivers/power/reset/reboot-mode.h >>>> create mode 100644 drivers/power/reset/syscon-reboot-mode.c >>>> >>>> diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig >>>> index 1131cf7..cf50630 100644 >>>> --- a/drivers/power/reset/Kconfig >>>> +++ b/drivers/power/reset/Kconfig >>>> @@ -173,5 +173,18 @@ config POWER_RESET_ZX >>>> help >>>> Reboot support for ZTE SoCs. >>>> +config REBOOT_MODE >>>> + tristate >>>> + >>>> +config SYSCON_REBOOT_MODE >>>> + bool "Generic SYSCON regmap reboot mode driver" >>>> + depends on OF >>>> + select REBOOT_MODE >>>> + help >>>> + Say y here will enable reboot mode driver. This will >>>> + get reboot mode arguments and store it in SYSCON mapped >>>> + register, then the bootloader can read it to take different >>>> + action according to the mode. >>>> + >>>> endif >>>> diff --git a/drivers/power/reset/Makefile >>>> b/drivers/power/reset/Makefile >>>> index 096fa67..a63865b 100644 >>>> --- a/drivers/power/reset/Makefile >>>> +++ b/drivers/power/reset/Makefile >>>> @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o >>>> obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o >>>> obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o >>>> obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o >>>> +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o >>>> +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o >>>> diff --git a/drivers/power/reset/reboot-mode.c >>>> b/drivers/power/reset/reboot-mode.c >>>> new file mode 100644 >>>> index 0000000..cdc4d72 >>>> --- /dev/null >>>> +++ b/drivers/power/reset/reboot-mode.c >>>> @@ -0,0 +1,118 @@ >>>> +/* >>>> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd >>>> + * >>>> + * This program is free software; you can redistribute it and/or modify >>>> + * it under the terms of the GNU General Public License as published by >>>> + * the Free Software Foundation; either version 2 of the License, or >>>> + * (at your option) any later version. >>>> + */ >>>> + >>>> +#include <linux/device.h> >>>> +#include <linux/init.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of.h> >>>> +#include <linux/reboot.h> >>>> +#include "reboot-mode.h" >>>> + >>>> +#define PREFIX "mode-" >>>> + >>>> +struct mode_info { >>>> + const char *mode; >>>> + unsigned int magic; >>>> + struct list_head list; >>>> +}; >>>> + >>>> +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, >>>> + const char *cmd) >>>> +{ >>>> + const char *normal = "normal"; >>>> + int magic = 0; >>>> + struct mode_info *info; >>>> + >>>> + if (!cmd) >>>> + cmd = normal; >>>> + >>>> + list_for_each_entry(info, &reboot->head, list) { >>>> + if (!strcmp(info->mode, cmd)) { >>>> + magic = info->magic; >>>> + break; >>>> + } >>>> + } >>>> + >>>> + return magic; >>>> +} >>>> + >>>> +static int reboot_mode_notify(struct notifier_block *this, >>>> + unsigned long mode, void *cmd) >>>> +{ >>>> + struct reboot_mode_driver *reboot; >>>> + int magic; >>>> + >>>> + reboot = container_of(this, struct reboot_mode_driver, >>>> reboot_notifier); >>>> + magic = get_reboot_mode_magic(reboot, cmd); >>>> + if (magic) >>>> + reboot->write(reboot, magic); >>>> + >>>> + return NOTIFY_DONE; >>>> +} >>>> + >>>> +int reboot_mode_register(struct reboot_mode_driver *reboot) >>>> +{ >>>> + struct mode_info *info; >>>> + struct property *prop; >>>> + struct device_node *np = reboot->dev->of_node; >>>> + size_t len = strlen(PREFIX); >>>> + int ret; >>>> + >>>> + INIT_LIST_HEAD(&reboot->head); >>>> + >>>> + for_each_property_of_node(np, prop) { >>>> + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, >>>> len)) >>>> + continue; >>>> + >>>> + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); >>>> + if (!info) { >>>> + ret = -ENOMEM; >>>> + goto error; >>>> + } >>>> + >>>> + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); >>>> + if (of_property_read_u32(np, prop->name, &info->magic)) { >>>> + dev_err(reboot->dev, "reboot mode %s without magic >>>> number\n", >>>> + info->mode); >>>> + devm_kfree(reboot->dev, info); >>> kfree_const(). The duplicated string won't be added to the list so it >>> won't be freed in error path or in unregister(). >> Each mode has a mode_info, if one of them gets an error whith >> devm_kzalloc, they other mode_info add to the list before should be freed. > Please, write it again. I don't understand. > > You are allocating 'info'. Then you are const-allocating 'info->mode'. > In this error path you are freeing 'info'. Where is the kfree_const() > for 'info->mode'? > >>>> + continue; >>>> + } >>>> + list_add_tail(&info->list, &reboot->head); >>>> + } >>>> + >>>> + reboot->reboot_notifier.notifier_call = reboot_mode_notify; >>>> + ret = register_reboot_notifier(&reboot->reboot_notifier); >>>> + if (ret) >>>> + dev_err(reboot->dev, "can't register reboot notifier\n"); >>> I don't understand your error paths. In previous patches they were >>> buggy... they still look buggy, I think. It's 8th iteration and such >>> basic things are still present. >>> >>> If this is an error then shouldn't everything be cleaned up? You are >>> returning error code thus the reboot_mode_register() caller will fail. >>> This means the probe will fail. So you need to clean up - go to error? >> If this is an error, I think only the duplicated string should be >> clean up,because other resources allocated by devm. If there is >> something else, please let me know. > Just goto error... I check the code of register_reboot_notifier, this function only return 0. So Maybe I don't need to check the return value here. > > Best regards, > Krzysztof > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Apr 24, 2016 at 11:55 PM, Andy Yan <andy.yan@rock-chips.com> wrote: [..] > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig [..] > +config SYSCON_REBOOT_MODE > + bool "Generic SYSCON regmap reboot mode driver" > + depends on OF As this isn't really useful without syscon (but still compiles), I would suggest adding: depends on SYSCON || COMPILE_TEST > + select REBOOT_MODE > + help > + Say y here will enable reboot mode driver. This will > + get reboot mode arguments and store it in SYSCON mapped > + register, then the bootloader can read it to take different > + action according to the mode. > + > endif > [..] > diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c [..] > + > +#define PREFIX "mode-" > + > +struct mode_info { > + const char *mode; > + unsigned int magic; If you're using of_property_read_u32() to populate this directly you should have this as u32. > + struct list_head list; > +}; > + > +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, > + const char *cmd) Magic comes from an unsigned property and is passed as unsigned to the regmap api. Please make it unsigned int throughout the implementation. > +{ > + const char *normal = "normal"; > + int magic = 0; > + struct mode_info *info; > + > + if (!cmd) > + cmd = normal; > + > + list_for_each_entry(info, &reboot->head, list) { > + if (!strcmp(info->mode, cmd)) { > + magic = info->magic; > + break; > + } > + } > + [..] > +int reboot_mode_register(struct reboot_mode_driver *reboot) > +{ > + struct mode_info *info; > + struct property *prop; > + struct device_node *np = reboot->dev->of_node; > + size_t len = strlen(PREFIX); > + int ret; > + > + INIT_LIST_HEAD(&reboot->head); > + > + for_each_property_of_node(np, prop) { > + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len)) There's no reason to iterate over the string twice here, strncmp will exit with -1 if you hit a '\0' before strlen(PREFIX). So you can drop the first check without any difference in functionality. What passes this checkout though is if the property equals PREFIX, but this is probably better to handle as an error, rather than skip over. So do that below > + continue; > + > + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); > + if (!info) { > + ret = -ENOMEM; > + goto error; > + } > + > + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); You need something like this here: if (!info->mode) { ret = -ENOMEM; goto error; } else if (info->mode[0] == '\0') dev_err("mode too short"); ret = -EINVAL; goto error; } If you do the kernel a favor and submit a patch to drivers/base/devres.c adding devm_kstrdup_const() you don't have to do the goto dance at all. > + if (of_property_read_u32(np, prop->name, &info->magic)) { > + dev_err(reboot->dev, "reboot mode %s without magic number\n", > + info->mode); > + devm_kfree(reboot->dev, info); > + continue; > + } > + list_add_tail(&info->list, &reboot->head); > + } > + > + reboot->reboot_notifier.notifier_call = reboot_mode_notify; > + ret = register_reboot_notifier(&reboot->reboot_notifier); > + if (ret) > + dev_err(reboot->dev, "can't register reboot notifier\n"); You're returning an error but haven't freed your info->modes. > + > + return ret; > + > +error: > + list_for_each_entry(info, &reboot->head, list) > + kfree_const(info->mode); > + > + return ret; > +} Regards, Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Mon, Apr 25, 2016 at 02:55:36PM +0800, Andy Yan wrote: > This driver parses the reboot commands like "reboot bootloader" > and "reboot recovery" to get a boot mode described in the > device tree , then call the write interfae to store the boot > mode in some place like special register or sram, which can > be read by the bootloader after system reboot, then the bootloader > can take different action according to the mode stored. > > This is commonly used on Android based devices, in order to > reboot the device into fastboot or recovery mode. > > Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> > Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> > Tested-by: John Stultz <john.stultz@linaro.org> > Acked-by: John Stultz <john.stultz@linaro.org> > Signed-off-by: Andy Yan <andy.yan@rock-chips.com> What's the status of this patchset? It still has a few open comments/issues, but I did not see a newer version. -- Sebastian
Hi Sebastian: On 2016年06月16日 22:36, Sebastian Reichel wrote: > Hi, > > On Mon, Apr 25, 2016 at 02:55:36PM +0800, Andy Yan wrote: >> This driver parses the reboot commands like "reboot bootloader" >> and "reboot recovery" to get a boot mode described in the >> device tree , then call the write interfae to store the boot >> mode in some place like special register or sram, which can >> be read by the bootloader after system reboot, then the bootloader >> can take different action according to the mode stored. >> >> This is commonly used on Android based devices, in order to >> reboot the device into fastboot or recovery mode. >> >> Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com> >> Reviewed-by: Moritz Fischer <moritz.fischer@ettus.com> >> Tested-by: John Stultz <john.stultz@linaro.org> >> Acked-by: John Stultz <john.stultz@linaro.org> >> Signed-off-by: Andy Yan <andy.yan@rock-chips.com> > What's the status of this patchset? It still has a few open > comments/issues, but I did not see a newer version. > > -- Sebastian Sorry for the delay, I will send a new version next week. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig index 1131cf7..cf50630 100644 --- a/drivers/power/reset/Kconfig +++ b/drivers/power/reset/Kconfig @@ -173,5 +173,18 @@ config POWER_RESET_ZX help Reboot support for ZTE SoCs. +config REBOOT_MODE + tristate + +config SYSCON_REBOOT_MODE + bool "Generic SYSCON regmap reboot mode driver" + depends on OF + select REBOOT_MODE + help + Say y here will enable reboot mode driver. This will + get reboot mode arguments and store it in SYSCON mapped + register, then the bootloader can read it to take different + action according to the mode. + endif diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile index 096fa67..a63865b 100644 --- a/drivers/power/reset/Makefile +++ b/drivers/power/reset/Makefile @@ -20,3 +20,5 @@ obj-$(CONFIG_POWER_RESET_SYSCON) += syscon-reboot.o obj-$(CONFIG_POWER_RESET_SYSCON_POWEROFF) += syscon-poweroff.o obj-$(CONFIG_POWER_RESET_RMOBILE) += rmobile-reset.o obj-$(CONFIG_POWER_RESET_ZX) += zx-reboot.o +obj-$(CONFIG_REBOOT_MODE) += reboot-mode.o +obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c new file mode 100644 index 0000000..cdc4d72 --- /dev/null +++ b/drivers/power/reset/reboot-mode.c @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/device.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/reboot.h> +#include "reboot-mode.h" + +#define PREFIX "mode-" + +struct mode_info { + const char *mode; + unsigned int magic; + struct list_head list; +}; + +static int get_reboot_mode_magic(struct reboot_mode_driver *reboot, + const char *cmd) +{ + const char *normal = "normal"; + int magic = 0; + struct mode_info *info; + + if (!cmd) + cmd = normal; + + list_for_each_entry(info, &reboot->head, list) { + if (!strcmp(info->mode, cmd)) { + magic = info->magic; + break; + } + } + + return magic; +} + +static int reboot_mode_notify(struct notifier_block *this, + unsigned long mode, void *cmd) +{ + struct reboot_mode_driver *reboot; + int magic; + + reboot = container_of(this, struct reboot_mode_driver, reboot_notifier); + magic = get_reboot_mode_magic(reboot, cmd); + if (magic) + reboot->write(reboot, magic); + + return NOTIFY_DONE; +} + +int reboot_mode_register(struct reboot_mode_driver *reboot) +{ + struct mode_info *info; + struct property *prop; + struct device_node *np = reboot->dev->of_node; + size_t len = strlen(PREFIX); + int ret; + + INIT_LIST_HEAD(&reboot->head); + + for_each_property_of_node(np, prop) { + if (len > strlen(prop->name) || strncmp(prop->name, PREFIX, len)) + continue; + + info = devm_kzalloc(reboot->dev, sizeof(*info), GFP_KERNEL); + if (!info) { + ret = -ENOMEM; + goto error; + } + + info->mode = kstrdup_const(prop->name + len, GFP_KERNEL); + if (of_property_read_u32(np, prop->name, &info->magic)) { + dev_err(reboot->dev, "reboot mode %s without magic number\n", + info->mode); + devm_kfree(reboot->dev, info); + continue; + } + list_add_tail(&info->list, &reboot->head); + } + + reboot->reboot_notifier.notifier_call = reboot_mode_notify; + ret = register_reboot_notifier(&reboot->reboot_notifier); + if (ret) + dev_err(reboot->dev, "can't register reboot notifier\n"); + + return ret; + +error: + list_for_each_entry(info, &reboot->head, list) + kfree_const(info->mode); + + return ret; +} + +int reboot_mode_unregister(struct reboot_mode_driver *reboot) +{ + struct mode_info *info; + + unregister_reboot_notifier(&reboot->reboot_notifier); + + list_for_each_entry(info, &reboot->head, list) + kfree_const(info->mode); + + return 0; +} + +MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com"); +MODULE_DESCRIPTION("System reboot mode driver"); +MODULE_LICENSE("GPL v2"); diff --git a/drivers/power/reset/reboot-mode.h b/drivers/power/reset/reboot-mode.h new file mode 100644 index 0000000..93ab703 --- /dev/null +++ b/drivers/power/reset/reboot-mode.h @@ -0,0 +1,14 @@ +#ifndef __REBOOT_MODE_H__ +#define __REBOOT_MODE_H__ + +struct reboot_mode_driver { + struct device *dev; + struct list_head head; + int (*write)(struct reboot_mode_driver *reboot, int magic); + struct notifier_block reboot_notifier; +}; + +int reboot_mode_register(struct reboot_mode_driver *reboot); +int reboot_mode_unregister(struct reboot_mode_driver *reboot); + +#endif diff --git a/drivers/power/reset/syscon-reboot-mode.c b/drivers/power/reset/syscon-reboot-mode.c new file mode 100644 index 0000000..5489dd9 --- /dev/null +++ b/drivers/power/reset/syscon-reboot-mode.c @@ -0,0 +1,99 @@ +/* + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/reboot.h> +#include <linux/regmap.h> +#include <linux/mfd/syscon.h> +#include "reboot-mode.h" + +struct syscon_reboot_mode { + struct regmap *map; + struct reboot_mode_driver reboot; + u32 offset; + u32 mask; +}; + +static int syscon_reboot_mode_write(struct reboot_mode_driver *reboot, + int magic) +{ + struct syscon_reboot_mode *syscon_rbm; + int ret; + + syscon_rbm = container_of(reboot, struct syscon_reboot_mode, reboot); + + ret = regmap_update_bits(syscon_rbm->map, syscon_rbm->offset, + syscon_rbm->mask, magic); + if (ret < 0) + dev_err(reboot->dev, "update reboot mode bits failed\n"); + + return ret; +} + +static int syscon_reboot_mode_probe(struct platform_device *pdev) +{ + int ret; + struct syscon_reboot_mode *syscon_rbm; + + syscon_rbm = devm_kzalloc(&pdev->dev, sizeof(*syscon_rbm), GFP_KERNEL); + if (!syscon_rbm) + return -ENOMEM; + + syscon_rbm->reboot.dev = &pdev->dev; + syscon_rbm->reboot.write = syscon_reboot_mode_write; + syscon_rbm->mask = 0xffffffff; + + dev_set_drvdata(&pdev->dev, syscon_rbm); + + syscon_rbm->map = syscon_node_to_regmap(pdev->dev.parent->of_node); + if (IS_ERR(syscon_rbm->map)) + return PTR_ERR(syscon_rbm->map); + + if (of_property_read_u32(pdev->dev.of_node, "offset", &syscon_rbm->offset)) + return -EINVAL; + + of_property_read_u32(pdev->dev.of_node, "mask", &syscon_rbm->mask); + + + ret = reboot_mode_register(&syscon_rbm->reboot); + if (ret) + dev_err(&pdev->dev, "can't register reboot mode\n"); + + return ret; +} + +static int syscon_reboot_mode_remove(struct platform_device *pdev) +{ + struct syscon_reboot_mode *syscon_rbm = dev_get_drvdata(&pdev->dev); + + return reboot_mode_unregister(&syscon_rbm->reboot); +} + +static const struct of_device_id syscon_reboot_mode_of_match[] = { + { .compatible = "syscon-reboot-mode" }, + {} +}; + +static struct platform_driver syscon_reboot_mode_driver = { + .probe = syscon_reboot_mode_probe, + .remove = syscon_reboot_mode_remove, + .driver = { + .name = "syscon-reboot-mode", + .of_match_table = syscon_reboot_mode_of_match, + }, +}; +module_platform_driver(syscon_reboot_mode_driver); + +MODULE_AUTHOR("Andy Yan <andy.yan@rock-chips.com"); +MODULE_DESCRIPTION("SYSCON reboot mode driver"); +MODULE_LICENSE("GPL v2");