Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Platform Driver <platform-driver-x86@vger.kernel.org>
Subject: Re: [PATCH v8 1/6] software node: rename is_array to is_inline
Date: Fri, 8 Nov 2019 10:49:38 +0100
Message-ID: <CAJZ5v0ghf58k_-dVZZGygQRBiVZuFfKUpFUsNK_0D15jvftRMw@mail.gmail.com> (raw)
In-Reply-To: <20191108042225.45391-2-dmitry.torokhov@gmail.com>

On Fri, Nov 8, 2019 at 5:22 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> We do not need a special flag to know if we are dealing with an array,
> as we can get that data from ratio between element length and the data
> size, however we do need a flag to know whether the data is stored
> directly inside property_entry or separately.

So the subject is slightly misleading, because it is not a rename.  I
would say "replace x with y" instead.

[Arguably I can change that when applying the patch, but since we are
going to wait for the dependencies to go in, it should not be a big
deal to send an update of this patch alone?]

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/base/swnode.c    | 12 +++++-------
>  include/linux/property.h | 13 ++++++++-----
>  2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index d8d0dc0ca5acf..18a30fb3cc588 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -108,10 +108,7 @@ static const void *property_get_pointer(const struct property_entry *prop)
>         if (!prop->length)
>                 return NULL;
>
> -       if (prop->is_array)
> -               return prop->pointer;
> -
> -       return &prop->value;
> +       return prop->is_inline ? &prop->value : prop->pointer;
>  }
>
>  static const void *property_entry_find(const struct property_entry *props,
> @@ -205,7 +202,7 @@ static void property_entry_free_data(const struct property_entry *p)
>         const char * const *src_str;
>         size_t i, nval;
>
> -       if (p->is_array) {
> +       if (!p->is_inline) {
>                 if (p->type == DEV_PROP_STRING && p->pointer) {
>                         src_str = p->pointer;
>                         nval = p->length / sizeof(const char *);
> @@ -250,7 +247,7 @@ static int property_entry_copy_data(struct property_entry *dst,
>         const void *pointer = property_get_pointer(src);
>         const void *new;
>
> -       if (src->is_array) {
> +       if (!src->is_inline) {
>                 if (!src->length)
>                         return -ENODATA;
>
> @@ -264,15 +261,16 @@ static int property_entry_copy_data(struct property_entry *dst,
>                                 return -ENOMEM;
>                 }
>
> -               dst->is_array = true;
>                 dst->pointer = new;
>         } else if (src->type == DEV_PROP_STRING) {
>                 new = kstrdup(src->value.str, GFP_KERNEL);
>                 if (!new && src->value.str)
>                         return -ENOMEM;
>
> +               dst->is_inline = true;
>                 dst->value.str = new;
>         } else {
> +               dst->is_inline = true;
>                 dst->value = src->value;
>         }
>
> diff --git a/include/linux/property.h b/include/linux/property.h
> index 48335288c2a96..dad0ad11b55e2 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -227,15 +227,17 @@ static inline int fwnode_property_count_u64(const struct fwnode_handle *fwnode,
>   * struct property_entry - "Built-in" device property representation.
>   * @name: Name of the property.
>   * @length: Length of data making up the value.
> - * @is_array: True when the property is an array.
> + * @is_inline: True when the property value is embedded in
> + *     &struct property_entry instance.
>   * @type: Type of the data in unions.
> - * @pointer: Pointer to the property (an array of items of the given type).
> - * @value: Value of the property (when it is a single item of the given type).
> + * @pointer: Pointer to the property when it is stored separately from
> + *     the &struct property_entry instance.
> + * @value: Value of the property when it is stored inline.

And while at it, can you please try to make the comments shorter so
they each take one line?

  reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08  4:22 [PATCH v8 0/6] software node: add support for reference properties Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 1/6] software node: rename is_array to is_inline Dmitry Torokhov
2019-11-08  9:49   ` Rafael J. Wysocki [this message]
2019-11-13  6:52   ` Bjørn Mork
2019-11-13  8:08     ` Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 2/6] software node: allow embedding of small arrays into property_entry Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 3/6] software node: implement reference properties Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 4/6] platform/x86: intel_cht_int33fe: use inline " Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 5/6] software node: remove separate handling of references Dmitry Torokhov
2019-11-08  4:22 ` [PATCH v8 6/6] software node: add basic tests for property entries Dmitry Torokhov

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJZ5v0ghf58k_-dVZZGygQRBiVZuFfKUpFUsNK_0D15jvftRMw@mail.gmail.com \
    --to=rafael@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org
	public-inbox-index linux-acpi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git