[PATCH v2 2/2] generic: platform/mikrotik: use MTD notifier
Thibaut
hacks at slashdirt.org
Tue Nov 16 08:27:25 PST 2021
LGTM
ACK
T
> Le 16 nov. 2021 à 17:06, Denis Kalashnikov <denis281089 at gmail.com> a écrit :
>
> If the SPI probe is sufficiently delayed, the routerboot driver may fail
> to init as the routerboot partitions are not yet available.
>
> Register an MTD user notifier instead of doing straight init so that the
> init subroutines are only executed when the target MTD partitions are
> present.
>
> Because the init/exit routines can now be called outside of the kernel
> normal init/exit calls, they cannot be jettisoned and must always be
> available: the __init and __exit qualifiers are thus removed.
>
> Reported-by: Denis Kalashnikov <denis281089 at gmail.com>
> Signed-off-by: Denis Kalashnikov <denis281089 at gmail.com>
> Signed-off-by: Thibaut VARÈNE <hacks at slashdirt.org>
> ---
>
> v2:
> * Use MTD notifier (hopefully a robust and right way of doing this thing)
> instead of late_initcall workaround.
>
> v1:
> * Replace module_init() with late_intcall() in routerboot module.
>
> .../drivers/platform/mikrotik/rb_hardconfig.c | 16 +++----
> .../drivers/platform/mikrotik/rb_softconfig.c | 16 +++----
> .../drivers/platform/mikrotik/routerboot.c | 45 ++++++++++++++++---
> .../drivers/platform/mikrotik/routerboot.h | 8 ++--
> 4 files changed, 59 insertions(+), 26 deletions(-)
>
> diff --git a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
> index e6a6928896..5ba8b376f2 100644
> --- a/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
> +++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_hardconfig.c
> @@ -676,10 +676,9 @@ static ssize_t hc_wlan_data_bin_read(struct file *filp, struct kobject *kobj,
> return count;
> }
>
> -int __init rb_hardconfig_init(struct kobject *rb_kobj)
> +int rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
> {
> struct kobject *hc_wlan_kobj;
> - struct mtd_info *mtd;
> size_t bytes_read, buflen, outlen;
> const u8 *buf;
> void *outbuf;
> @@ -690,20 +689,19 @@ int __init rb_hardconfig_init(struct kobject *rb_kobj)
> hc_kobj = NULL;
> hc_wlan_kobj = NULL;
>
> - // TODO allow override
> - mtd = get_mtd_device_nm(RB_MTD_HARD_CONFIG);
> - if (IS_ERR(mtd))
> + ret = __get_mtd_device(mtd);
> + if (ret)
> return -ENODEV;
>
> hc_buflen = mtd->size;
> hc_buf = kmalloc(hc_buflen, GFP_KERNEL);
> if (!hc_buf) {
> - put_mtd_device(mtd);
> + __put_mtd_device(mtd);
> return -ENOMEM;
> }
>
> ret = mtd_read(mtd, 0, hc_buflen, &bytes_read, hc_buf);
> - put_mtd_device(mtd);
> + __put_mtd_device(mtd);
>
> if (ret)
> goto fail;
> @@ -818,8 +816,10 @@ fail:
> return ret;
> }
>
> -void __exit rb_hardconfig_exit(void)
> +void rb_hardconfig_exit(void)
> {
> kobject_put(hc_kobj);
> + hc_kobj = NULL;
> kfree(hc_buf);
> + hc_buf = NULL;
> }
> diff --git a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
> index 070bd32d5a..5273b8d7ed 100644
> --- a/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
> +++ b/target/linux/generic/files/drivers/platform/mikrotik/rb_softconfig.c
> @@ -705,9 +705,8 @@ mtdfail:
>
> static struct kobj_attribute sc_kattrcommit = __ATTR(commit, RB_SC_RMODE|RB_SC_WMODE, sc_commit_show, sc_commit_store);
>
> -int __init rb_softconfig_init(struct kobject *rb_kobj)
> +int rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd)
> {
> - struct mtd_info *mtd;
> size_t bytes_read, buflen;
> const u8 *buf;
> int i, ret;
> @@ -716,20 +715,19 @@ int __init rb_softconfig_init(struct kobject *rb_kobj)
> sc_buf = NULL;
> sc_kobj = NULL;
>
> - // TODO allow override
> - mtd = get_mtd_device_nm(RB_MTD_SOFT_CONFIG);
> - if (IS_ERR(mtd))
> + ret = __get_mtd_device(mtd);
> + if (ret)
> return -ENODEV;
>
> sc_buflen = mtd->size;
> sc_buf = kmalloc(sc_buflen, GFP_KERNEL);
> if (!sc_buf) {
> - put_mtd_device(mtd);
> + __put_mtd_device(mtd);
> return -ENOMEM;
> }
>
> ret = mtd_read(mtd, 0, sc_buflen, &bytes_read, sc_buf);
> - put_mtd_device(mtd);
> + __put_mtd_device(mtd);
>
> if (ret)
> goto fail;
> @@ -799,8 +797,10 @@ fail:
> return ret;
> }
>
> -void __exit rb_softconfig_exit(void)
> +void rb_softconfig_exit(void)
> {
> kobject_put(sc_kobj);
> + sc_kobj = NULL;
> kfree(sc_buf);
> + sc_buf = NULL;
> }
> diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
> index 4c8c0bfac5..96f2460916 100644
> --- a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
> +++ b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.c
> @@ -13,6 +13,7 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/sysfs.h>
> +#include <linux/mtd/mtd.h>
>
> #include "routerboot.h"
>
> @@ -160,25 +161,57 @@ fail:
> return ret;
> }
>
> -static int __init routerboot_init(void)
> +static void routerboot_mtd_notifier_add(struct mtd_info *mtd)
> {
> - rb_kobj = kobject_create_and_add("mikrotik", firmware_kobj);
> - if (!rb_kobj)
> - return -ENOMEM;
> + /* Currently routerboot is only known to live on NOR flash */
> + if (mtd->type != MTD_NORFLASH)
> + return;
>
> /*
> * We ignore the following return values and always register.
> * These init() routines are designed so that their failed state is
> * always manageable by the corresponding exit() calls.
> + * Notifier is called with MTD mutex held: use __get/__put variants.
> + * TODO: allow partition names override
> */
> - rb_hardconfig_init(rb_kobj);
> - rb_softconfig_init(rb_kobj);
> + if (!strcmp(mtd->name, RB_MTD_HARD_CONFIG))
> + rb_hardconfig_init(rb_kobj, mtd);
> + else if (!strcmp(mtd->name, RB_MTD_SOFT_CONFIG))
> + rb_softconfig_init(rb_kobj, mtd);
> +}
> +
> +static void routerboot_mtd_notifier_remove(struct mtd_info *mtd)
> +{
> + if (mtd->type != MTD_NORFLASH)
> + return;
> +
> + if (!strcmp(mtd->name, RB_MTD_HARD_CONFIG))
> + rb_hardconfig_exit();
> + else if (!strcmp(mtd->name, RB_MTD_SOFT_CONFIG))
> + rb_softconfig_exit();
> +}
> +
> +/* Note: using a notifier prevents qualifying init()/exit() functions with __init/__exit */
> +static struct mtd_notifier routerboot_mtd_notifier = {
> + .add = routerboot_mtd_notifier_add,
> + .remove = routerboot_mtd_notifier_remove,
> +};
> +
> +static int __init routerboot_init(void)
> +{
> + rb_kobj = kobject_create_and_add("mikrotik", firmware_kobj);
> + if (!rb_kobj)
> + return -ENOMEM;
> +
> + register_mtd_user(&routerboot_mtd_notifier);
>
> return 0;
> }
>
> static void __exit routerboot_exit(void)
> {
> + unregister_mtd_user(&routerboot_mtd_notifier);
> + /* Exit routines are idempotent */
> rb_softconfig_exit();
> rb_hardconfig_exit();
> kobject_put(rb_kobj); // recursive afaict
> diff --git a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
> index 67d89808d5..e858a524af 100644
> --- a/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
> +++ b/target/linux/generic/files/drivers/platform/mikrotik/routerboot.h
> @@ -25,11 +25,11 @@
> int routerboot_tag_find(const u8 *bufhead, const size_t buflen, const u16 tag_id, u16 *pld_ofs, u16 *pld_len);
> int routerboot_rle_decode(const u8 *in, size_t inlen, u8 *out, size_t *outlen);
>
> -int __init rb_hardconfig_init(struct kobject *rb_kobj);
> -void __exit rb_hardconfig_exit(void);
> +int rb_hardconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
> +void rb_hardconfig_exit(void);
>
> -int __init rb_softconfig_init(struct kobject *rb_kobj);
> -void __exit rb_softconfig_exit(void);
> +int rb_softconfig_init(struct kobject *rb_kobj, struct mtd_info *mtd);
> +void rb_softconfig_exit(void);
>
> ssize_t routerboot_tag_show_string(const u8 *pld, u16 pld_len, char *buf);
> ssize_t routerboot_tag_show_u32s(const u8 *pld, u16 pld_len, char *buf);
> --
> 2.31.1
>
More information about the openwrt-devel
mailing list