Re: [PATCH] ARM: dts: Gateworks GW5520 support (i.MX6)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Sat, Aug 9, 2014 at 4:44 AM, Javier Martinez Canillas
<javier@xxxxxxxxxxxx> wrote:
> Hello Tim,
>
> On Sat, Aug 9, 2014 at 10:23 AM, Tim Harvey <tharvey@xxxxxxxxxxxxx> wrote:
>> +/*
>> + * Copyright 2014 Gateworks Corporation
>> + *
>> + * The code contained herein is licensed under the GNU General Public
>> + * License. You may obtain a copy of the GNU General Public License
>> + * Version 2 or later at the following locations:
>> + *
>> + * http://www.opensource.org/licenses/gpl-license.html
>> + * http://www.gnu.org/copyleft/gpl.html
>> + */
>
> There was a discussion [0] recently about what license(s) should be
> used for Device Tree source files. It seems that we have been using
> GPLv2-only just because is the Linux kernel license but unlike other
> source files, the DTS could be used verbatim in other operating
> systems / bootloaders so we have to better think the implications of
> the licensing.
>
> Of course this shouldn't hold your patch, I'm just mentioning so you
> can keep an eye in case an agreement is made while you are pushing
> this.

Javier,

Thanks for pointing this out - The discussion makes sense. I'll try to
keep an eye on it.

>
>> +
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +
>> +/ {
>> +       /* these are used by bootloader for disabling nodes */
>> +       aliases {
>> +               led0 = &led0;
>> +               led1 = &led1;
>> +               led2 = &led2;
>> +               nand = &gpmi;
>> +               usb0 = &usbh1;
>> +               usb1 = &usbotg;
>> +       };
>> +
>> +       chosen {
>> +               bootargs = "console=ttymxc1,115200";
>> +       };
>> +
>> +       leds {
>> +               compatible = "gpio-leds";
>> +               pinctrl-names = "default";
>> +               pinctrl-0 = <&pinctrl_gpio_leds>;
>> +
>> +               led0: user1 {
>> +                       label = "user1";
>> +                       gpios = <&gpio4 6 0>; /* MX6_PANLEDG */
>
> You are already including <dt-bindings/gpio/gpio.h> so please use the
> GPIO constants instead of magic numbers (e.g: GPIO_ACTIVE_HIGH in this
> case). Same for others gpios properties.

Agreed - I will change this for v2.

>
>> +                       default-state = "on";
>> +                       linux,default-trigger = "heartbeat";
>> +               };
>> +
>> +               led1: user2 {
>> +                       label = "user2";
>> +                       gpios = <&gpio4 7 0>; /* MX6_PANLEDR */
>> +                       default-state = "off";
>> +               };
>> +
>> +               led2: user3 {
>> +                       label = "user3";
>> +                       gpios = <&gpio4 15 1>; /* MX6_LOCLED# */
>> +                       default-state = "off";
>> +               };
>> +       };
>> +
>
> Overall, looks good to me. After the changes mentioned above:
>
> Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxx>
>
> [0]: http://www.spinics.net/lists/devicetree/msg44439.html

Thanks for the review.

Regards,

Tim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux