Dear John&All,

thanks for the review, see my response inline

On 08/29/2017 09:37 AM, John Crispin wrote:
> Hi,
>
> comment inline
>
>
> On 26/08/17 21:54, Zoltan Gyarmati wrote:
>>   The RT5350F's second UART pins are available on the base module and on
>> the EVB as well, so enable it in the device tree.
>>   Additionaly, the uartlite@c00 and uart@500 nodes swapped in
>> rt5350.dtsi
>> to keep the serial console as ttyS0.
>>
>> Signed-off-by: Zoltan Gyarmati <mr.zoltan.gyarm...@gmail.com>
>> ---
>>   target/linux/ramips/dts/RT5350F-OLINUXINO.dtsi | 11 +++++++++-
>>   target/linux/ramips/dts/rt5350.dtsi            | 30
>> +++++++++++++-------------
>>   2 files changed, 25 insertions(+), 16 deletions(-)
>>
>> diff --git a/target/linux/ramips/dts/RT5350F-OLINUXINO.dtsi
>> b/target/linux/ramips/dts/RT5350F-OLINUXINO.dtsi
>> index 955a13cddd..1632f3c085 100644
>> --- a/target/linux/ramips/dts/RT5350F-OLINUXINO.dtsi
>> +++ b/target/linux/ramips/dts/RT5350F-OLINUXINO.dtsi
>> @@ -46,9 +46,13 @@
>>   &pinctrl {
>>       state_default: pinctrl0 {
>>           gpio {
>> -            ralink,group = "jtag", "rgmii", "mdio", "uartf";
>> +            ralink,group = "jtag", "rgmii", "mdio";
>>               ralink,function = "gpio";
>>           };
>> +        uartf_gpio {
>> +            ralink,group = "uartf";
>> +            ralink,function = "gpio uartf";
>> +        };
>>       };
>>   };
>>   @@ -77,3 +81,8 @@
>>   &i2c {
>>       status = "okay";
>>   };
>> +
>> +&uart {
>> +    status = "okay";
>> +};
>> +
>> diff --git a/target/linux/ramips/dts/rt5350.dtsi
>> b/target/linux/ramips/dts/rt5350.dtsi
>> index a92c113043..f027e17d9d 100644
>> --- a/target/linux/ramips/dts/rt5350.dtsi
>> +++ b/target/linux/ramips/dts/rt5350.dtsi
>> @@ -83,21 +83,6 @@
>>               interrupts = <3>;
>>           };
>>   -        uart: uart@500 {
>> -            compatible = "ralink,rt5350-uart", "ralink,rt2880-uart",
>> "ns16550a";
>> -            reg = <0x500 0x100>;
>> -
>> -            resets = <&rstctrl 12>;
>> -            reset-names = "uart";
>> -
>> -            interrupt-parent = <&intc>;
>> -            interrupts = <5>;
>> -
>> -            reg-shift = <2>;
>> -
>> -            status = "disabled";
>> -        };
>> -
>>           gpio0: gpio@600 {
>>               compatible = "ralink,rt5350-gpio", "ralink,rt2880-gpio";
>>               reg = <0x600 0x34>;
>> @@ -221,6 +206,21 @@
>>               reg-shift = <2>;
>>           };
>>   +        uart: uart@500 {
>> +            compatible = "ralink,rt5350-uart", "ralink,rt2880-uart",
>> "ns16550a";
>> +            reg = <0x500 0x100>;
>> +
>> +            resets = <&rstctrl 12>;
>> +            reset-names = "uart";
>> +
>> +            interrupt-parent = <&intc>;
>> +            interrupts = <5>;
>> +
>> +            reg-shift = <2>;
>> +
>> +            status = "disabled";
>> +        };
>
> moving the node makes no sense. the change is a no-op. please remove
> it and update the patch description


I've just double checked it now, and the order of these nodes does
change the order of the serial port numbering, therefore it's not no-op.
Please see the relevant sections from both dmesg outputs:

#### With original rt5350.dtsi, uart@500 enabled
[    0.565407] gpio-export gpio_export: 3 gpio(s) exported
[    0.576247] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    0.590942] console [ttyS0] disabled
[    0.598130] 10000500.uart: ttyS0 at MMIO 0x10000500 (irq = 13,
base_baud = 2500000) is a Palmchip BK-3103
[    0.617160] console [ttyS0] enabled
[    0.630939] bootconsole [early0] disabled
[    0.647968] 10000c00.uartlite: ttyS1 at MMIO 0x10000c00 (irq = 20,
base_baud = 2500000) is a Palmchip BK-3103
[    0.680207] spi spi0.0: force spi mode3

#### With swapped items in rt5350.dtsi
[    0.564356] gpio-export gpio_export: 3 gpio(s) exported
[    0.575201] Serial: 8250/16550 driver, 2 ports, IRQ sharing disabled
[    0.590006] console [ttyS0] disabled
[    0.597183] 10000c00.uartlite: ttyS0 at MMIO 0x10000c00 (irq = 20,
base_baud = 2500000) is a Palmchip BK-3103
[    0.616906] console [ttyS0] enabled
[    0.630692] bootconsole [early0] disabled
[    0.647677] 10000500.uart: ttyS1 at MMIO 0x10000500 (irq = 13,
base_baud = 2500000) is a Palmchip BK-3103
[    0.678972] spi spi0.0: force spi mode3

Consequently (given that ttyS0 is configured as console in the kernel
command line),
the serial console moves to the pins of uart500 in the second (swapped)
case.
 Do you have any suggestion how to solve this on the level of
RT5350F-OLINUXINO.dtsi, without touching rt5350.dtsi?


>
>     John
>
>> +
>>           systick: systick@d00 {
>>               compatible = "ralink,rt5350-systick",
>> "ralink,cevt-systick";
>>               reg = <0xd00 0x10>;
>

Thanks, regards,

Zoltan Gyarmati
https://zgyarmati.de


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

Reply via email to