All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Mirza Krak <mirza.krak@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	Alexandre Courbot <gnurou@gmail.com>,
	pdeschrijver@nvidia.com, Prashant Gaikwad <pgaikwad@nvidia.com>,
	Michael Turquette <mturquette@baylibre.com>,
	sboyd@codeaurora.org, devicetree@vger.kernel.org,
	linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
	robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com,
	ijc+devicetree@hellion.org.uk, Kumar Gala <galak@codeaurora.org>,
	linux@armlinux.org.uk
Subject: Re: [RFC 6/6] bus: Add support for Tegra NOR controller
Date: Mon, 25 Jul 2016 15:41:38 +0200	[thread overview]
Message-ID: <20160725134138.GL21170@ulmo.ba.sec> (raw)
In-Reply-To: <CALw8SCU5s8BAg6B8dT=QokY-D-CcokmMEkYqz1GJWHkX-XWpRQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2000 bytes --]

On Mon, Jul 25, 2016 at 02:17:11PM +0200, Mirza Krak wrote:
> 2016-07-25 13:14 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Tue, Jul 19, 2016 at 03:36:37PM +0200, Mirza Krak wrote:
[...]
> >> +
> >> +     if (of_get_child_count(of_node))
> >> +             ret = of_platform_populate(of_node,
> >> +                                of_default_bus_match_table,
> >> +                                NULL, &pdev->dev);
> >
> > Why the extra check? of_platform_populate() is almost a no-op if there
> > aren't any children anyway. What it will do is set the OF_POPULATED_BUS
> > flag, but I think we want that irrespective of whether or not there are
> > any children.
> >
> > Was there any problem with calling it unconditionally that made you opt
> > for the extra check?
> 
> I have not tested calling it unconditionally. Used another driver as
> reference and they had that condition (drivers/bus/imx-weim.c), that
> driver does not mention why the condition is there. But will test
> removing the condition and see what happens.

If that works fine for Tegra, it might be a good idea to get rid of the
call in the imx-weim.c driver, too.

> >> +static struct platform_driver nor_driver = {
> >> +     .driver = {
> >> +             .name           = "tegra-nor",
> >> +             .of_match_table = nor_id_table,
> >> +     },
> >> +};
> >> +module_platform_driver_probe(nor_driver, nor_probe);
> >> +
> >> +MODULE_AUTHOR("Mirza Krak <mirza.krak@gmail.com");
> >> +MODULE_DESCRIPTION("NVIDIA Tegra NOR Bus Driver");
> >> +MODULE_LICENSE("GPL");
> >
> > You're header comment says GPL version 2, which means that the
> > MODULE_LICENSE needs to be "GPL v2". "GPL" means "version 2 or later".
> 
> Ok, I do not really mind it being GPL version 2 or later so will
> change the header comment instead if that is ok.

I think "GPL v2" is traditionally more common in kernel code, but as the
author the decision is obviously yours.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Mirza Krak <mirza.krak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>,
	Alexandre Courbot
	<gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org,
	Prashant Gaikwad
	<pgaikwad-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Michael Turquette
	<mturquette-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>,
	sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org
Subject: Re: [RFC 6/6] bus: Add support for Tegra NOR controller
Date: Mon, 25 Jul 2016 15:41:38 +0200	[thread overview]
Message-ID: <20160725134138.GL21170@ulmo.ba.sec> (raw)
In-Reply-To: <CALw8SCU5s8BAg6B8dT=QokY-D-CcokmMEkYqz1GJWHkX-XWpRQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2060 bytes --]

On Mon, Jul 25, 2016 at 02:17:11PM +0200, Mirza Krak wrote:
> 2016-07-25 13:14 GMT+02:00 Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> > On Tue, Jul 19, 2016 at 03:36:37PM +0200, Mirza Krak wrote:
[...]
> >> +
> >> +     if (of_get_child_count(of_node))
> >> +             ret = of_platform_populate(of_node,
> >> +                                of_default_bus_match_table,
> >> +                                NULL, &pdev->dev);
> >
> > Why the extra check? of_platform_populate() is almost a no-op if there
> > aren't any children anyway. What it will do is set the OF_POPULATED_BUS
> > flag, but I think we want that irrespective of whether or not there are
> > any children.
> >
> > Was there any problem with calling it unconditionally that made you opt
> > for the extra check?
> 
> I have not tested calling it unconditionally. Used another driver as
> reference and they had that condition (drivers/bus/imx-weim.c), that
> driver does not mention why the condition is there. But will test
> removing the condition and see what happens.

If that works fine for Tegra, it might be a good idea to get rid of the
call in the imx-weim.c driver, too.

> >> +static struct platform_driver nor_driver = {
> >> +     .driver = {
> >> +             .name           = "tegra-nor",
> >> +             .of_match_table = nor_id_table,
> >> +     },
> >> +};
> >> +module_platform_driver_probe(nor_driver, nor_probe);
> >> +
> >> +MODULE_AUTHOR("Mirza Krak <mirza.krak-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org");
> >> +MODULE_DESCRIPTION("NVIDIA Tegra NOR Bus Driver");
> >> +MODULE_LICENSE("GPL");
> >
> > You're header comment says GPL version 2, which means that the
> > MODULE_LICENSE needs to be "GPL v2". "GPL" means "version 2 or later".
> 
> Ok, I do not really mind it being GPL version 2 or later so will
> change the header comment instead if that is ok.

I think "GPL v2" is traditionally more common in kernel code, but as the
author the decision is obviously yours.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: thierry.reding@gmail.com (Thierry Reding)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 6/6] bus: Add support for Tegra NOR controller
Date: Mon, 25 Jul 2016 15:41:38 +0200	[thread overview]
Message-ID: <20160725134138.GL21170@ulmo.ba.sec> (raw)
In-Reply-To: <CALw8SCU5s8BAg6B8dT=QokY-D-CcokmMEkYqz1GJWHkX-XWpRQ@mail.gmail.com>

On Mon, Jul 25, 2016 at 02:17:11PM +0200, Mirza Krak wrote:
> 2016-07-25 13:14 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>:
> > On Tue, Jul 19, 2016 at 03:36:37PM +0200, Mirza Krak wrote:
[...]
> >> +
> >> +     if (of_get_child_count(of_node))
> >> +             ret = of_platform_populate(of_node,
> >> +                                of_default_bus_match_table,
> >> +                                NULL, &pdev->dev);
> >
> > Why the extra check? of_platform_populate() is almost a no-op if there
> > aren't any children anyway. What it will do is set the OF_POPULATED_BUS
> > flag, but I think we want that irrespective of whether or not there are
> > any children.
> >
> > Was there any problem with calling it unconditionally that made you opt
> > for the extra check?
> 
> I have not tested calling it unconditionally. Used another driver as
> reference and they had that condition (drivers/bus/imx-weim.c), that
> driver does not mention why the condition is there. But will test
> removing the condition and see what happens.

If that works fine for Tegra, it might be a good idea to get rid of the
call in the imx-weim.c driver, too.

> >> +static struct platform_driver nor_driver = {
> >> +     .driver = {
> >> +             .name           = "tegra-nor",
> >> +             .of_match_table = nor_id_table,
> >> +     },
> >> +};
> >> +module_platform_driver_probe(nor_driver, nor_probe);
> >> +
> >> +MODULE_AUTHOR("Mirza Krak <mirza.krak@gmail.com");
> >> +MODULE_DESCRIPTION("NVIDIA Tegra NOR Bus Driver");
> >> +MODULE_LICENSE("GPL");
> >
> > You're header comment says GPL version 2, which means that the
> > MODULE_LICENSE needs to be "GPL v2". "GPL" means "version 2 or later".
> 
> Ok, I do not really mind it being GPL version 2 or later so will
> change the header comment instead if that is ok.

I think "GPL v2" is traditionally more common in kernel code, but as the
author the decision is obviously yours.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160725/daba8235/attachment.sig>

  reply	other threads:[~2016-07-25 13:41 UTC|newest]

Thread overview: 133+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-19 13:36 [RFC 0/6] Add support for Tegra20/30 NOR bus controller Mirza Krak
2016-07-19 13:36 ` Mirza Krak
2016-07-19 13:36 ` [RFC 1/6] clk: tegra: add TEGRA20_CLK_NOR to init table Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-25 11:17   ` Thierry Reding
2016-07-25 11:17     ` Thierry Reding
2016-07-25 12:28     ` Mirza Krak
2016-07-25 12:28       ` Mirza Krak
2016-07-25 12:28       ` Mirza Krak
2016-07-25 13:23       ` Thierry Reding
2016-07-25 13:23         ` Thierry Reding
2016-07-25 13:23         ` Thierry Reding
2016-07-19 13:36 ` [RFC 2/6] clk: tegra: add TEGRA30_CLK_NOR " Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36 ` [RFC 3/6] dt/bindings: Add bindings for Tegra20/30 NOR bus driver Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-20 12:44   ` Rob Herring
2016-07-20 12:44     ` Rob Herring
2016-07-20 12:44     ` Rob Herring
2016-07-20 19:28     ` Mirza Krak
2016-07-20 19:28       ` Mirza Krak
2016-07-20 19:28       ` Mirza Krak
2016-07-21 10:26       ` Jon Hunter
2016-07-21 10:26         ` Jon Hunter
2016-07-21 10:26         ` Jon Hunter
2016-07-25 11:36         ` Thierry Reding
2016-07-25 11:36           ` Thierry Reding
2016-07-25 13:20           ` Mirza Krak
2016-07-25 13:20             ` Mirza Krak
2016-07-25 13:27             ` Thierry Reding
2016-07-25 13:27               ` Thierry Reding
2016-07-25 13:33               ` Mirza Krak
2016-07-25 13:33                 ` Mirza Krak
2016-07-21  9:56   ` Jon Hunter
2016-07-21  9:56     ` Jon Hunter
2016-07-21  9:56     ` Jon Hunter
2016-07-21 20:10     ` Mirza Krak
2016-07-21 20:10       ` Mirza Krak
2016-07-22  9:32       ` Jon Hunter
2016-07-22  9:32         ` Jon Hunter
2016-07-22  9:32         ` Jon Hunter
2016-07-22 19:07         ` Mirza Krak
2016-07-22 19:07           ` Mirza Krak
2016-07-25  8:14           ` Jon Hunter
2016-07-25  8:14             ` Jon Hunter
2016-07-25  8:14             ` Jon Hunter
2016-07-25 12:10       ` Thierry Reding
2016-07-25 12:10         ` Thierry Reding
2016-07-25 12:10         ` Thierry Reding
2016-07-25 13:09         ` Jon Hunter
2016-07-25 13:09           ` Jon Hunter
2016-07-25 13:09           ` Jon Hunter
2016-07-25 13:32           ` Thierry Reding
2016-07-25 13:32             ` Thierry Reding
2016-07-25 11:59     ` Thierry Reding
2016-07-25 11:59       ` Thierry Reding
2016-07-25 13:30       ` Mirza Krak
2016-07-25 13:30         ` Mirza Krak
2016-07-25 13:30         ` Mirza Krak
2016-07-25 13:39         ` Thierry Reding
2016-07-25 13:39           ` Thierry Reding
2016-07-25 13:39           ` Thierry Reding
2016-07-25 13:50           ` Mirza Krak
2016-07-25 13:50             ` Mirza Krak
2016-07-25 13:50             ` Mirza Krak
2016-07-25 13:36       ` Jon Hunter
2016-07-25 13:36         ` Jon Hunter
2016-07-25 13:36         ` Jon Hunter
2016-07-25 13:49         ` Thierry Reding
2016-07-25 13:49           ` Thierry Reding
2016-07-25 11:30   ` Thierry Reding
2016-07-25 11:30     ` Thierry Reding
2016-07-25 11:30     ` Thierry Reding
2016-07-25 13:16     ` Mirza Krak
2016-07-25 13:16       ` Mirza Krak
2016-07-25 14:15       ` Thierry Reding
2016-07-25 14:15         ` Thierry Reding
2016-07-25 14:38         ` Mirza Krak
2016-07-25 14:38           ` Mirza Krak
2016-07-25 15:01           ` Jon Hunter
2016-07-25 15:01             ` Jon Hunter
2016-07-25 15:01             ` Jon Hunter
2016-07-25 15:34             ` Thierry Reding
2016-07-25 15:34               ` Thierry Reding
2016-07-25 15:34               ` Thierry Reding
2016-07-25 19:59         ` Mirza Krak
2016-07-25 19:59           ` Mirza Krak
2016-07-26  8:32           ` Thierry Reding
2016-07-26  8:32             ` Thierry Reding
2016-07-26  8:32             ` Thierry Reding
2016-07-28  9:29         ` Mirza Krak
2016-07-28  9:29           ` Mirza Krak
2016-07-28  9:29           ` Mirza Krak
2016-07-19 13:36 ` [RFC 4/6] ARM: tegra: Add Tegra30 NOR support Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36 ` [RFC 5/6] ARM: tegra: Add Tegra20 " Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36 ` [RFC 6/6] bus: Add support for Tegra NOR controller Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-19 13:36   ` Mirza Krak
2016-07-21 10:15   ` Jon Hunter
2016-07-21 10:15     ` Jon Hunter
2016-07-21 10:15     ` Jon Hunter
2016-07-21 20:42     ` Mirza Krak
2016-07-21 20:42       ` Mirza Krak
2016-07-21 20:42       ` Mirza Krak
2016-07-22  9:38       ` Jon Hunter
2016-07-22  9:38         ` Jon Hunter
2016-07-22  9:38         ` Jon Hunter
2016-07-22 19:18         ` Mirza Krak
2016-07-22 19:18           ` Mirza Krak
2016-07-25  8:19           ` Jon Hunter
2016-07-25  8:19             ` Jon Hunter
2016-07-25  8:19             ` Jon Hunter
2016-07-25 10:57           ` Thierry Reding
2016-07-25 10:57             ` Thierry Reding
2016-07-21 15:12   ` Jon Hunter
2016-07-21 15:12     ` Jon Hunter
2016-07-21 15:12     ` Jon Hunter
2016-07-21 21:41     ` Mirza Krak
2016-07-21 21:41       ` Mirza Krak
2016-07-25 11:14   ` Thierry Reding
2016-07-25 11:14     ` Thierry Reding
2016-07-25 12:17     ` Mirza Krak
2016-07-25 12:17       ` Mirza Krak
2016-07-25 13:41       ` Thierry Reding [this message]
2016-07-25 13:41         ` Thierry Reding
2016-07-25 13:41         ` Thierry Reding

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=20160725134138.GL21170@ulmo.ba.sec \
    --to=thierry.reding@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=gnurou@gmail.com \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=mirza.krak@gmail.com \
    --cc=mturquette@baylibre.com \
    --cc=pawel.moll@arm.com \
    --cc=pdeschrijver@nvidia.com \
    --cc=pgaikwad@nvidia.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=swarren@wwwdotorg.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
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.