Re: [linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
On 19 April 2016 at 17:11, Chen-Yu Tsai wrote: > On Tue, Apr 19, 2016 at 10:46 PM, wrote: >> Hi ChenYu, >> >> Thanks for your comments. >> >> On Tuesday, April 19, 2016 at 7:11:50 AM UTC-4, Chen-Yu Tsai wrote: >>> Hi, >>> >>> > arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 36 ++ >>> > arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 36 ++ >>> > arch/arm/boot/dts/sun8i-h3.dtsi| 75 >>> >>> First of all, you are touching 3 different files here. These should >>> be separate patches. >> >> I'm trying to understand you here, but I can't. Those 3 files changed are >> related each other. I could have separated the UART changes from I2C >> changes, but still those 3 files would have been modified at the same time >> for a single commit and "git patch-format" would still have created a single >> patch for the 3 files commit. > > Try to wrap lines to 80 characters or less. > > 1 change per patch. You are making 3 changes here. a) adding shared > pinmux settings, > b & c) enabling uarts and i2c busses for 2 boards. > > You could split them into 1 dtsi patch adding the pinmux setting, and > then 1 for each > board enabling the peripherals. > >> >> Seeing all the patches that coming into the mailing lists, all of them >> contains multiple files patches, why should it be different here ? >> >>> >>> Secondly, our policy is to not have a default function for generic GPIO >>> pins. >>> >> >> If this is the official policy, then why so many DTS currently present are >> not following the same rules, such sun6i-a31-hummingbird, >> sun7i-a20-olinuxino-micro, sun7i-a20-mk808c, sun7i-a20-cubietruck and so >> many others ? > > Perhaps they were enabled before the policy was enacted, or it just > slipped through. I > contributed to a few. But for many boards we might not have schematics > or the actual > device to check. > > Also, other boards having such settings is not a good argument. +1 for what Wen is saying here. If the pins go to a header then it's best to leave it as a GPIO. Uart2 on the mk808c is enabled as it's used for Bluetooth. CK > >> I thought the rules were there to make DTS the most default common usage >> definitions for most end-users in a general availability. >> Then, if someone is really in shortage of GPIOs, they could easily turn them >> back to "disabled" state. > > How would you determine what the most common usage is for "development > boards"? > If the vendor explicitly designed the pins to be used one way, and even > printed > the description on the board, then you may have a valid argument. But even > then, > with the C.H.I.P., they are going with DT overlays. > > It shouldn't be hard for an end user to modify the DT. And with I2C devices, > it > is almost a requirement. > > > Regards > ChenYu > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
Oupps ! sorry for again have a line length exceeding 80 chars. Regards, -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
On Tuesday, April 19, 2016 at 11:11:37 AM UTC-4, Chen-Yu Tsai wrote: > Try to wrap lines to 80 characters or less. Ok ! I will be more careful ... > 1 change per patch. You are making 3 changes here. a) adding shared > pinmux settings, > b & c) enabling uarts and i2c busses for 2 boards. > > You could split them into 1 dtsi patch adding the pinmux setting, and > then 1 for each > board enabling the peripherals. Fine ! I will prepare and redo a new submit with only the DTSI for now. > Perhaps they were enabled before the policy was enacted, or it just > slipped through. I > contributed to a few. But for many boards we might not have schematics > or the actual > device to check. Understood ! But sometimes things are so generic, like having uart0 and not having uart1 make it looks a bit strange. Some other times having some i2c2 because of PMIC while not having i2c0 and i2c1 is also strange. > How would you determine what the most common usage is for "development > boards"? > If the vendor explicitly designed the pins to be used one way, and even > printed > the description on the board, then you may have a valid argument. But even > then, > with the C.H.I.P., they are going with DT overlays. > > It shouldn't be hard for an end user to modify the DT. And with I2C devices, > it > is almost a requirement. Ok ! I will look if it is easier to do the rest with overlays. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
On Tue, Apr 19, 2016 at 10:46 PM, wrote: > Hi ChenYu, > > Thanks for your comments. > > On Tuesday, April 19, 2016 at 7:11:50 AM UTC-4, Chen-Yu Tsai wrote: >> Hi, >> >> > arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 36 ++ >> > arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 36 ++ >> > arch/arm/boot/dts/sun8i-h3.dtsi| 75 >> >> First of all, you are touching 3 different files here. These should >> be separate patches. > > I'm trying to understand you here, but I can't. Those 3 files changed are > related each other. I could have separated the UART changes from I2C changes, > but still those 3 files would have been modified at the same time for a > single commit and "git patch-format" would still have created a single patch > for the 3 files commit. Try to wrap lines to 80 characters or less. 1 change per patch. You are making 3 changes here. a) adding shared pinmux settings, b & c) enabling uarts and i2c busses for 2 boards. You could split them into 1 dtsi patch adding the pinmux setting, and then 1 for each board enabling the peripherals. > > Seeing all the patches that coming into the mailing lists, all of them > contains multiple files patches, why should it be different here ? > >> >> Secondly, our policy is to not have a default function for generic GPIO pins. >> > > If this is the official policy, then why so many DTS currently present are > not following the same rules, such sun6i-a31-hummingbird, > sun7i-a20-olinuxino-micro, sun7i-a20-mk808c, sun7i-a20-cubietruck and so many > others ? Perhaps they were enabled before the policy was enacted, or it just slipped through. I contributed to a few. But for many boards we might not have schematics or the actual device to check. Also, other boards having such settings is not a good argument. > I thought the rules were there to make DTS the most default common usage > definitions for most end-users in a general availability. > Then, if someone is really in shortage of GPIOs, they could easily turn them > back to "disabled" state. How would you determine what the most common usage is for "development boards"? If the vendor explicitly designed the pins to be used one way, and even printed the description on the board, then you may have a valid argument. But even then, with the C.H.I.P., they are going with DT overlays. It shouldn't be hard for an end user to modify the DT. And with I2C devices, it is almost a requirement. Regards ChenYu -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
Hi ChenYu, Thanks for your comments. On Tuesday, April 19, 2016 at 7:11:50 AM UTC-4, Chen-Yu Tsai wrote: > Hi, > > > arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 36 ++ > > arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 36 ++ > > arch/arm/boot/dts/sun8i-h3.dtsi| 75 > > First of all, you are touching 3 different files here. These should > be separate patches. I'm trying to understand you here, but I can't. Those 3 files changed are related each other. I could have separated the UART changes from I2C changes, but still those 3 files would have been modified at the same time for a single commit and "git patch-format" would still have created a single patch for the 3 files commit. Seeing all the patches that coming into the mailing lists, all of them contains multiple files patches, why should it be different here ? > > Secondly, our policy is to not have a default function for generic GPIO pins. > If this is the official policy, then why so many DTS currently present are not following the same rules, such sun6i-a31-hummingbird, sun7i-a20-olinuxino-micro, sun7i-a20-mk808c, sun7i-a20-cubietruck and so many others ? I thought the rules were there to make DTS the most default common usage definitions for most end-users in a general availability. Then, if someone is really in shortage of GPIOs, they could easily turn them back to "disabled" state. Regards, Martin. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
[linux-sunxi] Re: [PATCH v1 1/1] add missing UARTs pins for AllWinner H3 DTS + add new I2C entries for AllWinner H3 DTS
Hi,
On Sat, Apr 16, 2016 at 1:11 AM, Martin Ayotte wrote:
> Hi everyone,
>
> This patch is submit to provide endusers access to additional UARTs on
> AllWinner H3 SoC along with I2C ports.
>
> Regards,
> Martin.
> ---
> arch/arm/boot/dts/sun8i-h3-orangepi-2.dts | 36 ++
> arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts | 36 ++
> arch/arm/boot/dts/sun8i-h3.dtsi| 75
First of all, you are touching 3 different files here. These should
be separate patches.
Secondly, our policy is to not have a default function for generic GPIO pins.
Regards
ChenYu
> ++
> 3 files changed, 147 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
> b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
> index f93f5d1..f0b9823 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
> @@ -176,6 +176,42 @@
> status = "okay";
> };
>
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins_a>;
> + status = "okay";
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins_a>;
> + status = "okay";
> +};
> +
> +&uart3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart3_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins_a>;
> + status = "okay";
> +};
> +
> &usb1_vbus_pin_a {
> allwinner,pins = "PG13";
> };
> diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> index daf50b9a6..6994349 100644
> --- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> +++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts
> @@ -161,6 +161,42 @@
> status = "okay";
> };
>
> +&uart1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart1_pins_a>;
> + status = "okay";
> +};
> +
> +&uart2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart2_pins_a>;
> + status = "okay";
> +};
> +
> +&uart3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&uart3_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins_a>;
> + status = "okay";
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins_a>;
> + status = "okay";
> +};
> +
> &usbphy {
> /* USB VBUS is always on */
> status = "okay";
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi
> b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 4a4926b..c947360d 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -508,6 +508,48 @@
> allwinner,pull = ;
> };
>
> + uart1_pins_a: uart1@0 {
> + allwinner,pins = "PG6", "PG7";
> + allwinner,function = "uart1";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + uart2_pins_a: uart2@0 {
> + allwinner,pins = "PA0", "PA1";
> + allwinner,function = "uart2";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + uart3_pins_a: uart3@0 {
> + allwinner,pins = "PA13", "PA14";
> + allwinner,function = "uart3";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + i2c0_pins_a: i2c0@0 {
> + allwinner,pins = "PA11", "PA12";
> + allwinner,function = "i2c0";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + i2c1_pins_a: i2c1@0 {
> + allwinner,pins = "PA18", "PA19";
> + allwinner,function = "i2c1";
> + allwinner,drive = ;
> + allwinner,pull = ;
> + };
> +
> + i2c2_pins_a: i2c2@0 {
> + allwinner,pins = "PE12", "PE13";
> + allwinner,function = "i2c2";
> + allwi
