[<prev] [next>] [day] [month] [year] [list]
Message-ID: <5994BE5B.5000706@gmail.com>
Date: Wed, 16 Aug 2017 14:51:23 -0700
From: Frank Rowand <frowand.list@...il.com>
To: Chris Mi <chrism@...lanox.com>
Cc: arm-soc <linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
Rob Herring <robh+dt@...nel.org>
Subject: Re: [patch net-next 0/3] net/sched: Improve getting objects by
indexes
I deleted most of the distribution list. My email server rejects an email
with this many recipients.
On 08/15/17 19:12, Chris Mi wrote:
> IDR uses internally radix tree which uses unsigned long. It doesn't
> makes sense to have index as signed value.
>
> Signed-off-by: Chris Mi <chrism@...lanox.com>
> Signed-off-by: Jiri Pirko <jiri@...lanox.com>
> ---
< snip >
> drivers/of/overlay.c | 15 +++----
> drivers/of/unittest.c | 25 ++++++-----
< snip >
> include/linux/of.h | 4 +-
< snip >
Split the patch apart.
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c0e4ee1..e5cfe01 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -373,10 +373,11 @@ static int of_free_overlay_info(struct of_overlay *ov)
> *
> * Returns the id of the created overlay, or a negative error number
> */
> -int of_overlay_create(struct device_node *tree)
> +int of_overlay_create(struct device_node *tree, unsigned long *id)
Added parameter *id, but never assigned a value to it.
> {
> struct of_overlay *ov;
> - int err, id;
> + unsigned long idr_index;
> + int err;
>
> /* allocate the overlay structure */
> ov = kzalloc(sizeof(*ov), GFP_KERNEL);
> @@ -390,12 +391,10 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_lock(&of_mutex);
>
> - id = idr_alloc(&ov_idr, ov, 0, 0, GFP_KERNEL);
> - if (id < 0) {
> - err = id;
> + err = idr_alloc(&ov_idr, ov, &idr_index, 0, 0, GFP_KERNEL);
> + if (err)
> goto err_destroy_trans;
> - }
> - ov->id = id;
> + ov->id = idr_index;
>
> /* build the overlay info structures */
> err = of_build_overlay_info(ov, tree);
> @@ -430,7 +429,7 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_unlock(&of_mutex);
>
> - return id;
> + return err;
>
> err_revert_overlay:
> err_abort_trans:
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 0107fc6..ac7cc76 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -1242,7 +1242,8 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> int *overlay_id)
> {
> struct device_node *np = NULL;
> - int ret, id = -1;
> + unsigned long id = -1;
Assigns a negative value to an unsigned.
> + int ret;
>
> np = of_find_node_by_path(overlay_path(overlay_nr));
> if (np == NULL) {
> @@ -1252,17 +1253,14 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> goto out;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr));
> goto out;
> }
> - id = ret;
> of_unittest_track_overlay(id);
>
> - ret = 0;
> -
> out:
> of_node_put(np);
>
> @@ -1442,6 +1440,7 @@ static void of_unittest_overlay_6(void)
> int ret, i, ov_id[2];
> int overlay_nr = 6, unittest_nr = 6;
> int before = 0, after = 1;
> + unsigned long id;
>
> /* unittest device must be in before state */
> for (i = 0; i < 2; i++) {
> @@ -1466,13 +1465,13 @@ static void of_unittest_overlay_6(void)
> return;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr + i));
> return;
> }
> - ov_id[i] = ret;
> + ov_id[i] = id;
> of_unittest_track_overlay(ov_id[i]);
> }
>
> @@ -2094,6 +2093,7 @@ static int __init overlay_data_add(int onum)
> int ret;
> u32 size;
> u32 size_from_header;
> + unsigned long id;
>
> for (k = 0, info = overlays; info; info++, k++) {
> if (k == onum)
> @@ -2138,13 +2138,12 @@ static int __init overlay_data_add(int onum)
> goto out_free_np_overlay;
> }
>
> - ret = of_overlay_create(info->np_overlay);
> - if (ret < 0) {
> + ret = of_overlay_create(info->np_overlay, &id);
> + if (ret) {
> pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum);
> goto out_free_np_overlay;
> } else {
> - info->overlay_id = ret;
> - ret = 0;
> + info->overlay_id = id;
> }
>
> pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4a8a709..ceb14bf 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1307,7 +1307,7 @@ struct of_overlay_notify_data {
> #ifdef CONFIG_OF_OVERLAY
>
> /* ID based overlays; the API for external users */
> -int of_overlay_create(struct device_node *tree);
> +int of_overlay_create(struct device_node *tree, *unsigned long *id);
*unsigned long *id should be: unsigned long *id
How did you test this patch?
> int of_overlay_destroy(int id);
> int of_overlay_destroy_all(void);
>
> @@ -1316,7 +1316,7 @@ struct of_overlay_notify_data {
>
> #else
>
> -static inline int of_overlay_create(struct device_node *tree)
> +static inline int of_overlay_create(struct device_node *tree, unsigned long *id)
> {
> return -ENOTSUPP;
> }
Powered by blists - more mailing lists