[PATCH v2 2/2] realtek: add support for Panasonic Switch-M8eG PN28080K

INAGAKI Hiroshi musashino.open at gmail.com
Sat Dec 4 19:36:39 PST 2021


Hi Sander,

Thank you for the review.

On 2021/12/05 1:02, Sander Vanheule wrote:
> Hi Hiroshi,
>
> Thanks for the update. Some inline comments below.
>
> On Tue, 2021-11-30 at 19:43 +0900, INAGAKI Hiroshi wrote:
>> Panasonic Switch-M8eG PN28080K is a 8 + 1 port gigabit switch, based on
>> RTL8380M.
>>
>> Specification:
>>
>> - SoC           : Realtek RTL8380M
>> - RAM           : DDR3 128 MiB (Winbond W631GG8KB-15)
>> - Flash         : SPI-NOR 32 MiB (Macronix MX25L25635FMI-10G)
>> - Ethernet      : 10/100/1000 Mbps x8 + 1
>>    - port 1-8    : TP, RTL8218B (SoC)
>>    - port 9      : SFP, RTL8380M (SoC)
>> - LEDs/Keys     : 7x / 1x
>> - UART          : RS-232 port on the front panel (connector: RJ-45)
>>    - 3:TX, 4:GND, 5:GND, 6:RX (pin number: RJ-45)
>>    - 9600n8
>> - Power         : 100-240 VAC, 50/60 Hz, 0.5 A
>>    - Plug        : IEC 60320-C13
>> - Stock OS      : VxWorks based
>>
>> Flash instruction using initramfs image:
>>
>> 1.  Prepare the TFTP server with the IP address 192.168.1.111
>> 2.  Rename the OpenWrt initramfs image to "0101A8C0.img" and place it to
>>      the TFTP directory
>> 3.  Download the official upgrading firmware (ex: pn28080k_v30000.rom)
>>      and place it to the TFTP directory
>> 4.  Boot M8eG and interrupt the U-Boot with Ctrl + C keys
>> 5.  Execute the following commands and boot with the OpenWrt initramfs
>>      image
>>
>>      rtk network on
>>      tftpboot 0x81000000
>>      bootm
>>
>> 6.  Backup mtdblock files to the computer by scp or anything and reboot
>> 7.  Interrupt the U-Boot and execute the following commands to re-create
>>      filesystem in the flash
>>
>>      ffsmount c:/
>>      ffsfmt c:/
>>
>>      this step takes a long time, about ~ 4 mins
>>
>> 8.  Execute the following commands to put the official images to the
>>      filesystem
>>
>>      updatert <official image>
>>
>>      example:
>>
>>        updatert pn28080k_v30000.rom
>>
>>      this step takes about ~ 40 secs
>>
>> 9.  Set the environment variables of the U-Boot by the following commands
>>
>>      setenv loadaddr 0xb4e00000
>>      setenv bootcmd bootm
>>      saveenv
>>
>> 10: Download the OpenWrt initramfs image and boot with it
>>
>>      tftpboot 0x81000000 0101A8C0.img
>>      bootm
>>
>> 11: On the initramfs image, download the sysupgrade image and perform
>>      sysupgrade with it
>>
>>      sysupgrade <imagename>
>>
>> 12: Wait ~ 120 seconds to complete flashing
>>
>> Note:
>>
>> - "Switch-M8eG" is a model name, and "PN28080K" is a model number.
>>    Switch-M8eG has an another (old) model number ("PN28080"), it's not a
>>    Realtek based hardware.
>>
>> - Switch-M8eG has a "POWER" LED (Green), but it's not connected to any
>>    GPIO pin.
>>
>> - The U-Boot checks the runtime images in the flash when booting and
>>    fails to execute anything in "bootcmd" variable if the images are not
>>    exsisting.
>>
>> - A filesystem is formed in the flash (0x100000-0x1DFFFFF) on the stock
>>    firmware and it includes the stock images, configuration files and
>>    checksum files. It's unknown format, can't be managed on the OpenWrt.
>>    To get the enough space for OpenWrt, move the filesystem to the head
>>    of "fs_reserved" partition by execution of "ffsfmt" and "updatert".
>>
>> - On the other devices in the same series of Switch-M8eG PN28080K, the
>>    INT pin on the PCA9555 is not connected to anywhere.
>>
>> Back to the stock firmware:
>>
>> 1. Delete "loadaddr" variable and set "bootcmd" to the original value
>>
>>     on U-Boot:
>>
>>       setenv loadaddr
>>       setenv bootcmd 'bootm 0x81000000'
>>
>>     on OpenWrt:
>>
>>       fw_setenv loadaddr
>>       fw_setenv bootcmd 'bootm 0x81000000'
>>
>> 2. Perform reset or reboot
>>
>>    on U-Boot:
>>
>>      reset
>>
>>    on OpenWrt:
>>
>>      reboot
>>
>> Signed-off-by: INAGAKI Hiroshi <musashino.open at gmail.com>
>> ---
>> v1 -> v2:
>>
>> - drop changes for kernel 5.4
>> - add interrupt-related properties to the nodes of PCA9539 and PCA9555
>>    - use "gpio-keys" instead of "gpio-keys-polled"
>> - update/remove comments in mtd partitions
>> - use "led-*" scheme and add color/function properties for each node of
>>    LEDs
>> - drop unnecessary default values from gpio-restart node
>>
>>   .../rtl8380_panasonic_m8eg-pn28080k.dts       | 123 ++++++++++
>>   .../rtl83xx_panasonic_mxxeg-pn28xx0k.dtsi     | 229 ++++++++++++++++++
>>   target/linux/realtek/image/Makefile           |  10 +
>>   3 files changed, 362 insertions(+)
>>   create mode 100644 target/linux/realtek/dts-5.10/rtl8380_panasonic_m8eg-
>> pn28080k.dts
>>   create mode 100644 target/linux/realtek/dts-5.10/rtl83xx_panasonic_mxxeg-
>> pn28xx0k.dtsi
>>
>> diff --git a/target/linux/realtek/dts-5.10/rtl8380_panasonic_m8eg-pn28080k.dts
>> b/target/linux/realtek/dts-5.10/rtl8380_panasonic_m8eg-pn28080k.dts
>> new file mode 100644
>> index 0000000000..7b6bfc7991
>> --- /dev/null
>> +++ b/target/linux/realtek/dts-5.10/rtl8380_panasonic_m8eg-pn28080k.dts
>> @@ -0,0 +1,123 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>> +
>> +#include "rtl838x.dtsi"
>> +#include "rtl83xx_panasonic_mxxeg-pn28xx0k.dtsi"
> I take it you chose to perform all the includes here because the larger switches
> are RTL839x-based, not RTL838x. I find strictly hierarchical includes typically
> easier to follow. Given the amount of common configuration in these devices, I
> can see the benefit of performing the includes here. But I'll let others decide
> over whether this is acceptable or not.


I see, I'll keep in mind.

>
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +
>> +/ {
>> +       compatible = "panasonic,m8eg-pn28080k", "realtek,rtl838x-soc";
>> +       model = "Panasonic Switch-M8eG PN28080K";
>> +
>> +       aliases {
>> +               led-boot = &led_status_eco_green;
>> +               led-failsafe = &led_status_eco_amber;
>> +               led-running = &led_status_eco_green;
>> +               led-upgrade = &led_status_eco_green;
>> +       };
>> +
>> +       sfp0: sfp-p9 {
>> +               compatible = "sff,sfp";
>> +               i2c-bus = <&i2c0>;
> You define all the i2cN busses in pn28xx0k DTSI, but not all busses are actually
> wired up on all switches. See my comment below with the I2C mux.
>
>> +               tx-fault-gpio = <&gpio1 0 GPIO_ACTIVE_HIGH>;
>> +               tx-disable-gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpio = <&gpio1 2 GPIO_ACTIVE_LOW>;
>> +               los-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
>> +       };
>> +};
>> +
>> +&leds {
>> +       led_status_eco_amber: led-5 {
>> +               label = "amber:status_eco";
>> +               gpios = <&gpio2 1 GPIO_ACTIVE_LOW>;
>> +               color = <LED_COLOR_ID_AMBER>;
>> +               function = LED_FUNCTION_STATUS;
>> +               function-enumerator = <1>;
>> +       };
>> +
>> +       led_status_eco_green: led-6 {
>> +               label = "green:status_eco";
>> +               gpios = <&gpio2 2 GPIO_ACTIVE_LOW>;
>> +               color = <LED_COLOR_ID_GREEN>;
>> +               function = LED_FUNCTION_STATUS;
>> +               function-enumerator = <2>;
>> +       };
>> +};
>> +
>> +&i2c_gpio_0 {
>> +       scl-gpios = <&gpio0 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +       sda-gpios = <&gpio0 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +};
>> +
>> +&i2c_gpio_1 {
>> +       scl-gpios = <&gpio0 12 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +       sda-gpios = <&gpio0 13 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>> +};
>> +
>> +&gpio1 {
>> +       interrupt-controller;
>> +       #interrupt-cells = <2>;
>> +       interrupt-parent = <&gpio0>;
>> +       interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
>> +};
>> +
>> +&gpio2 {
>> +       interrupt-parent = <&gpio0>;
>> +       interrupts = <2 IRQ_TYPE_EDGE_FALLING>;
> Could you also group all interrupt-* properties here, like you do for gpio1?



Indeed, I'll update it in the next patch series.

>
> [...]
>
>> diff --git a/target/linux/realtek/dts-5.10/rtl83xx_panasonic_mxxeg-
>> pn28xx0k.dtsi b/target/linux/realtek/dts-5.10/rtl83xx_panasonic_mxxeg-
>> pn28xx0k.dtsi
>> new file mode 100644
>> index 0000000000..40bf3a264e
>> --- /dev/null
>> +++ b/target/linux/realtek/dts-5.10/rtl83xx_panasonic_mxxeg-pn28xx0k.dtsi
>> @@ -0,0 +1,229 @@
> [...]
>
>> +
>> +       i2c_gpio_1: i2c-gpio-1 {
>> +               compatible = "i2c-gpio";
>> +               i2c-gpio,delay-us = <2>;
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +
>> +               i2c-switch at 70 {
>> +                       compatible = "nxp,pca9545";
>> +                       reset-gpios = <&gpio2 13 GPIO_ACTIVE_LOW>;
>> +                       reg = <0x70>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +
>> +                       /*
>> +                        * - Switch-M8eG  PN28080K: 0   (sfp p9)
>> +                        * - Switch-M24eG PN28240K: 0-1 (sfp p23-p24)
>> +                        * - Switch-M48eG PN28480K: 0-3 (sfp p45-p48)
>> +                        */
> Since these busses aren't actually used here, I would suggest to add a label to
> i2c-switch at 70, and then define the required bus(ses) in the device DTS.

I've defined all the buses in dtsi because the driver creates the 
maximum number supported by the chip regardless of the number of buses 
defined. But as you said, dts/dtsi should accurately indicate the 
specifications of the device.
I'll update it in the next patch series.

>
>> +                       i2c0: i2c at 0 {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +                               reg = <0>;
>> +                       };
>> +
>> +                       i2c1: i2c at 1 {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +                               reg = <1>;
>> +                       };
>> +
>> +                       i2c2: i2c at 2 {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +                               reg = <2>;
>> +                       };
>> +
>> +                       i2c3: i2c at 3 {
>> +                               #address-cells = <1>;
>> +                               #size-cells = <0>;
>> +                               reg = <3>;
>> +                       };
>> +               };
>> +       };
>> +};
>
> --
> Best,
> Sander

Regards,
Hiroshi



More information about the openwrt-devel mailing list