All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
To: Mark Brown <broonie@kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"Alexander Holler" <holler@ahsoftware.de>,
	"Alexandre Courbot" <gnurou@gmail.com>,
	"Andrzej Hajda" <a.hajda@samsung.com>,
	"Arnd Bergmann" <arnd@arndb.de>,
	"Dmitry Torokhov" <dmitry.torokhov@gmail.com>,
	"Grant Likely" <grant.likely@linaro.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Ian Campbell" <ijc+devicetree@hellion.org.uk>,
	"Javier Martinez Canillas" <javier.martinez@collabora.co.uk>,
	"Krzysztof Kozlowski" <k.kozlowski@samsung.com>,
	"Kumar Gala" <galak@codeaurora.org>,
	"Len Brown" <lenb@kernel.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Lv Zheng" <lv.zheng@intel.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Pawel Moll" <pawel.moll@arm.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	"Robert Moore" <robert.moore@intel.com>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Russell King" <linux@arm.linux.org.uk>,
	"Stephen Warren" <swarren@wwwdotorg.org>,
	"Terje Bergström" <tbergstrom@nvidia.com>,
	"Thierry Reding" <thierry.reding@gmail.com>
Subject: Re: [PATCH 08/13] gpio: sysfs: implement class.get_dependencies()
Date: Tue, 30 Jun 2015 17:00:53 +0200	[thread overview]
Message-ID: <CAAObsKDYvgxuxH0ZpXaTrsjLYOaZ-W_evOWfLLTNFRFYMra5LQ@mail.gmail.com> (raw)
In-Reply-To: <20150617174009.GZ14071@sirena.org.uk>

On 17 June 2015 at 19:40, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jun 17, 2015 at 03:42:18PM +0200, Tomeu Vizoso wrote:
>
>> +static bool strends(const char *str, const char *postfix)
>> +{
>> +     if (strlen(str) < strlen(postfix))
>> +             return false;
>> +
>> +     return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
>> +}
>
> This is named like (and looks like) a generic fuction, shouldn't it be
> in string.h or something?

Yeah, will put it there.

>> +static void add_dependency(struct fwnode_handle *fwnode,
>> +                        struct list_head *list)
>> +{
>> +     struct fwnode_dependency *dep;
>> +
>> +     dep = kzalloc(sizeof(*dep), GFP_KERNEL);
>> +     if (!dep)
>> +             return;
>> +
>> +     INIT_LIST_HEAD(&dep->dependency);
>> +     dep->fwnode = fwnode;
>> +
>> +     list_add_tail(&dep->dependency, list);
>> +}
>
> Might be worth putting this in generic code, it looks pretty generic?  I
> have to say I'm unclear what frees the returned list.

Agreed.

>> +     if (!is_of_node(fwnode))
>> +             return NULL;
>> +
>> +     np = of_node(fwnode);
>> +     if (!np)
>> +             return NULL;
>
> Presumably the first check could be dropped?

That's right.

>> +     list = kzalloc(sizeof(*list), GFP_KERNEL);
>> +     if (!list)
>> +             return NULL;
>
> Might it make sense for the core to allocate the head of the list and
> just ask the classes to add to the list?  We're going to want to merge
> the dependencies from multiple subsystems and that saves allocating
> heads that may never get anything added to them.

Yes, I have gone with that advice and it looks better that way.

Thanks,

Tomeu

WARNING: multiple messages have this Message-ID (diff)
From: tomeu.vizoso@collabora.com (Tomeu Vizoso)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 08/13] gpio: sysfs: implement class.get_dependencies()
Date: Tue, 30 Jun 2015 17:00:53 +0200	[thread overview]
Message-ID: <CAAObsKDYvgxuxH0ZpXaTrsjLYOaZ-W_evOWfLLTNFRFYMra5LQ@mail.gmail.com> (raw)
In-Reply-To: <20150617174009.GZ14071@sirena.org.uk>

On 17 June 2015 at 19:40, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jun 17, 2015 at 03:42:18PM +0200, Tomeu Vizoso wrote:
>
>> +static bool strends(const char *str, const char *postfix)
>> +{
>> +     if (strlen(str) < strlen(postfix))
>> +             return false;
>> +
>> +     return strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
>> +}
>
> This is named like (and looks like) a generic fuction, shouldn't it be
> in string.h or something?

Yeah, will put it there.

>> +static void add_dependency(struct fwnode_handle *fwnode,
>> +                        struct list_head *list)
>> +{
>> +     struct fwnode_dependency *dep;
>> +
>> +     dep = kzalloc(sizeof(*dep), GFP_KERNEL);
>> +     if (!dep)
>> +             return;
>> +
>> +     INIT_LIST_HEAD(&dep->dependency);
>> +     dep->fwnode = fwnode;
>> +
>> +     list_add_tail(&dep->dependency, list);
>> +}
>
> Might be worth putting this in generic code, it looks pretty generic?  I
> have to say I'm unclear what frees the returned list.

Agreed.

>> +     if (!is_of_node(fwnode))
>> +             return NULL;
>> +
>> +     np = of_node(fwnode);
>> +     if (!np)
>> +             return NULL;
>
> Presumably the first check could be dropped?

That's right.

>> +     list = kzalloc(sizeof(*list), GFP_KERNEL);
>> +     if (!list)
>> +             return NULL;
>
> Might it make sense for the core to allocate the head of the list and
> just ask the classes to add to the list?  We're going to want to merge
> the dependencies from multiple subsystems and that saves allocating
> heads that may never get anything added to them.

Yes, I have gone with that advice and it looks better that way.

Thanks,

Tomeu

  reply	other threads:[~2015-06-30 15:01 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 13:42 [PATCH 00/13] Discover and probe dependencies Tomeu Vizoso
2015-06-17 13:42 ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 01/13] gpiolib: Fix docs for gpiochip_add_pingroup_range Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-07-13 12:16   ` Linus Walleij
2015-07-13 12:16     ` Linus Walleij
2015-06-17 13:42 ` [PATCH 02/13] driver-core: defer all probes until late_initcall Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-18 21:50   ` Rafael J. Wysocki
2015-06-18 21:50     ` Rafael J. Wysocki
2015-06-19 13:36     ` Tomeu Vizoso
2015-06-19 13:36       ` Tomeu Vizoso
2015-06-19 23:20       ` Rafael J. Wysocki
2015-06-19 23:20         ` Rafael J. Wysocki
2015-06-23  0:07         ` Rob Herring
2015-06-23  0:07           ` Rob Herring
2015-06-23 14:37           ` Rafael J. Wysocki
2015-06-23 14:37             ` Rafael J. Wysocki
2015-06-23 14:17             ` Tomeu Vizoso
2015-06-23 14:17               ` Tomeu Vizoso
2015-06-23 14:51               ` Rafael J. Wysocki
2015-06-23 14:51                 ` Rafael J. Wysocki
2015-06-23 14:37                 ` Tomeu Vizoso
2015-06-23 14:37                   ` Tomeu Vizoso
2015-06-24  0:14                   ` Rafael J. Wysocki
2015-06-24  0:14                     ` Rafael J. Wysocki
2015-06-17 13:42 ` [PATCH 03/13] ARM: tegra: Add gpio-ranges property Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 17:25   ` Mark Brown
2015-06-17 17:25     ` Mark Brown
2015-06-18  8:06     ` Tomeu Vizoso
2015-06-18  8:06       ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 04/13] pinctrl: tegra: Only set the gpio range if needed Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-07-13 20:14   ` Linus Walleij
2015-07-13 20:14     ` Linus Walleij
2015-07-14  8:34     ` Tomeu Vizoso
2015-07-14  8:34       ` Tomeu Vizoso
2015-07-15  3:17       ` Alexandre Courbot
2015-07-15  3:17         ` Alexandre Courbot
2015-07-15  8:13         ` Tomeu Vizoso
2015-07-15  8:13           ` Tomeu Vizoso
2015-07-17  8:04       ` Linus Walleij
2015-07-17  8:04         ` Linus Walleij
2015-07-17  8:19         ` Tomeu Vizoso
2015-07-17  8:19           ` Tomeu Vizoso
2015-07-17  9:36           ` Linus Walleij
2015-07-17  9:36             ` Linus Walleij
2015-06-17 13:42 ` [PATCH 05/13] driver core: fix docbook for device_private.device Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 06/13] of/platform: Set fwnode field for new devices Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 17:27   ` Mark Brown
2015-06-17 17:27     ` Mark Brown
2015-06-17 13:42 ` [PATCH 07/13] driver-core: Add class.get_dependencies() callback Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 08/13] gpio: sysfs: implement class.get_dependencies() Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 17:40   ` Mark Brown
2015-06-17 17:40     ` Mark Brown
2015-06-30 15:00     ` Tomeu Vizoso [this message]
2015-06-30 15:00       ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 09/13] gpu: host1x: " Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 10/13] driver-core: add for_each_class() Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 11/13] device property: add fwnode_get_parent() Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 12/13] device property: add fwnode_get_name() Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 13:42 ` [PATCH 13/13] driver-core: probe dependencies before probing Tomeu Vizoso
2015-06-17 13:42   ` Tomeu Vizoso
2015-06-17 18:13   ` Mark Brown
2015-06-17 18:13     ` Mark Brown
2015-06-30 15:18     ` Tomeu Vizoso
2015-06-30 15:18       ` Tomeu Vizoso
2015-06-18  9:42 ` [PATCH 00/13] Discover and probe dependencies Andrzej Hajda
2015-06-18  9:42   ` Andrzej Hajda
2015-06-18  9:57   ` Russell King - ARM Linux
2015-06-18  9:57     ` Russell King - ARM Linux
2015-06-18 10:36   ` Mark Brown
2015-06-18 10:36     ` Mark Brown
2015-06-18 13:14     ` Andrzej Hajda
2015-06-18 13:14       ` Andrzej Hajda
2015-06-18 14:38       ` Tomeu Vizoso
2015-06-18 14:38         ` Tomeu Vizoso
2015-06-18 14:49       ` Russell King - ARM Linux
2015-06-18 14:49         ` Russell King - ARM Linux
2015-06-18 15:32         ` Alexander Holler
2015-06-18 15:32           ` Alexander Holler
2015-06-18 14:57   ` Tomeu Vizoso
2015-06-18 14:57     ` Tomeu Vizoso

Reply instructions:

You may reply publicly 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=CAAObsKDYvgxuxH0ZpXaTrsjLYOaZ-W_evOWfLLTNFRFYMra5LQ@mail.gmail.com \
    --to=tomeu.vizoso@collabora.com \
    --cc=a.hajda@samsung.com \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=grant.likely@linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=holler@ahsoftware.de \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=javier.martinez@collabora.co.uk \
    --cc=k.kozlowski@samsung.com \
    --cc=lenb@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=lv.zheng@intel.com \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=rjw@rjwysocki.net \
    --cc=robert.moore@intel.com \
    --cc=robh+dt@kernel.org \
    --cc=swarren@wwwdotorg.org \
    --cc=tbergstrom@nvidia.com \
    --cc=thierry.reding@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.