lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 04 Mar 2022 12:15:21 +0100
From:   Michael Walle <michael@...le.cc>
To:     Claudiu.Beznea@...rochip.com
Cc:     Kavyasree.Kotagiri@...rochip.com, Nicolas.Ferre@...rochip.com,
        arnd@...db.de, olof@...om.net, soc@...nel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        krzysztof.kozlowski@...onical.com, alexandre.belloni@...tlin.com
Subject: Re: [PATCH v1 6/6] ARM: dts: lan966x: add basic Kontron KSwitch D10
 support

Am 2022-03-04 09:31, schrieb Claudiu.Beznea@...rochip.com:
> On 03.03.2022 18:03, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Add basic support for the Kontron KSwitch D10 MMT 6G-2GS which
>> features 6 Gigabit copper ports and two SFP cages. For now the
>> following is working:
>>  - Kernel console
>>  - SFP cages I2C bus and mux
>>  - SPI
>>  - SGPIO
>>  - Watchdog
>> 
>> Signed-off-by: Michael Walle <michael@...le.cc>
>> ---
>>  arch/arm/boot/dts/Makefile                    |   3 +-
>>  ...lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts | 159 
>> ++++++++++++++++++
>>  2 files changed, 161 insertions(+), 1 deletion(-)
>>  create mode 100644 
>> arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> 
>> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
>> index 085c43649d44..86dd0f9804ee 100644
>> --- a/arch/arm/boot/dts/Makefile
>> +++ b/arch/arm/boot/dts/Makefile
>> @@ -739,7 +739,8 @@ dtb-$(CONFIG_SOC_IMX7ULP) += \
>>         imx7ulp-com.dtb \
>>         imx7ulp-evk.dtb
>>  dtb-$(CONFIG_SOC_LAN966) += \
>> -       lan966x-pcb8291.dtb
>> +       lan966x-pcb8291.dtb \
>> +       lan966x-kontron-kswitch-d10-mmt-6g-2gs.dtb
>>  dtb-$(CONFIG_SOC_LS1021A) += \
>>         ls1021a-moxa-uc-8410a.dtb \
>>         ls1021a-qds.dtb \
>> diff --git 
>> a/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts 
>> b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> new file mode 100644
>> index 000000000000..958678dec7ad
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt-6g-2gs.dts
>> @@ -0,0 +1,159 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Device Tree file for the Kontron KSwitch D10 MMT 6G-2GS
>> + */
>> +
>> +/dts-v1/;
>> +#include "lan966x.dtsi"
>> +
>> +/ {
>> +       model = "Kontron KSwitch D10 MMT 6G-2GS";
>> +       compatible = "kontron,kswitch-d10-mmt-6g-2gs", 
>> "kontron,s1921",
>> +                    "microchip,lan9668", "microchip,lan966";
>> +
>> +       aliases {
>> +               serial0 = &usart0;
>> +       };
>> +
>> +       chosen {
>> +               stdout-path = "serial0:115200n8";
>> +       };
>> +
>> +       gpio-restart {
>> +               compatible = "gpio-restart";
>> +               gpios = <&gpio 56 GPIO_ACTIVE_LOW>;
>> +               priority = <200>;
>> +       };
>> +
>> +       i2cmux {
>> +               compatible = "i2c-mux-gpio";
>> +               #address-cells = <1>;
>> +               #size-cells = <0>;
>> +               mux-gpios = <&sgpio_out 3 2 GPIO_ACTIVE_HIGH>,
>> +                           <&sgpio_out 3 3 GPIO_ACTIVE_HIGH>;
>> +               i2c-parent = <&i2c4>;
>> +
>> +               i2c4_0: i2c@1 {
>> +                       reg = <1>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +               };
>> +
>> +               i2c4_1: i2c@2 {
>> +                       reg = <2>;
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +               };
>> +       };
>> +
>> +       sfp0: sfp0 {
>> +               compatible = "sff,sfp";
>> +               i2c-bus = <&i2c4_0>;
>> +               los-gpios = <&sgpio_in 1 0 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpios = <&sgpio_in 1 1 GPIO_ACTIVE_LOW>;
>> +               maximum-power-milliwatt = <2500>;
>> +               tx-disable-gpios = <&sgpio_out 3 0 GPIO_ACTIVE_LOW>;
>> +               tx-fault-gpios = <&sgpio_in 0 2 GPIO_ACTIVE_HIGH>;
>> +               rate-select0-gpios = <&sgpio_out 2 0 
>> GPIO_ACTIVE_HIGH>;
>> +               rate-select1-gpios = <&sgpio_out 2 1 
>> GPIO_ACTIVE_HIGH>;
>> +       };
>> +
>> +       sfp1: sfp1 {
>> +               compatible = "sff,sfp";
>> +               i2c-bus = <&i2c4_1>;
>> +               los-gpios = <&sgpio_in 1 2 GPIO_ACTIVE_HIGH>;
>> +               mod-def0-gpios = <&sgpio_in 1 3 GPIO_ACTIVE_LOW>;
>> +               maximum-power-milliwatt = <2500>;
>> +               tx-disable-gpios = <&sgpio_out 3 1 GPIO_ACTIVE_LOW>;
>> +               tx-fault-gpios = <&sgpio_in 0 3 GPIO_ACTIVE_HIGH>;
>> +               rate-select0-gpios = <&sgpio_out 2 2 
>> GPIO_ACTIVE_HIGH>;
>> +               rate-select1-gpios = <&sgpio_out 2 3 
>> GPIO_ACTIVE_HIGH>;
>> +       };
>> +};
>> +
>> +&flx0 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_USART>;
>> +       status = "okay";
>> +};
>> +
>> +&flx3 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_SPI>;
>> +       status = "okay";
>> +};
>> +
>> +&flx4 {
>> +       atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
>> +       status = "okay";
>> +};
> 
> Although there is 1:1 mapping b/w ids of flexcoms and the embedded 
> blocks
> (flxX has usartX, i2cX, spiX) and there is nothing wrong with the 
> approach
> here I found a bit hard to follow if the correspondent embedded block
> (i2c, spi, usart) is enabled or not.

I know and I had the same feeling, but I don't want to have the
subnodes (matched by name) in these nodes.  I.e. I want to avoid
something like:

&flx4 {
        atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>;
        status = "okay";

        i2c@600 {
               pinctrl-0 = <&fc4_b_pins>;
               pinctrl-names = "default";
               status = "okay";
        };
};

If someone renames the subnode in the dtsi, it might easily be
overlooked in the board files. Having the handle will raise an
error.

And because the node references should be sorted alphabetically
it will be cluttered around in the file. You could rename the
references to flx4_i2c though. But I don't know it its worth
the efforts. Let me know what you think.

-michael

> 
>> +
>> +&gpio {
>> +       usart0_pins: usart0-pins {
>> +               /* RXD, TXD */
>> +               pins = "GPIO_25", "GPIO_26";
>> +               function = "fc0_b";
>> +       };
>> +
>> +       sgpio_a_pins: sgpio-a-pins {
>> +               /* SCK, D0, D1, LD */
>> +               pins = "GPIO_32", "GPIO_33", "GPIO_34";
>> +               function = "sgpio_a";
>> +       };
>> +
>> +       sgpio_b_pins: sgpio-b-pins {
>> +               /* SCK, D0, D1, LD */
>> +               pins = "GPIO_64";
>> +               function = "sgpio_b";
>> +       };
>> +
>> +       fc3_b_pins: fc3-b-spi-pins {
>> +               /* SCK, MISO, MOSI */
>> +               pins = "GPIO_51", "GPIO_52", "GPIO_53";
>> +               function = "fc3_b";
>> +       };
>> +
>> +       fc4_b_pins: fc4-b-i2c-pins {
>> +               /* RXD, TXD */
>> +               pins = "GPIO_57", "GPIO_58";
>> +               function = "fc4_b";
>> +       };
>> +};
>> +
>> +&i2c4 {
>> +       pinctrl-0 = <&fc4_b_pins>;
>> +       pinctrl-names = "default";
>> +       status = "okay";
>> +};
>> +
>> +&usart0 {
>> +       pinctrl-0 = <&usart0_pins>;
>> +       pinctrl-names = "default";
>> +       status = "okay";
>> +};
>> +
>> +&sgpio {
>> +       pinctrl-0 = <&sgpio_a_pins>, <&sgpio_b_pins>;
>> +       pinctrl-names = "default";
>> +       bus-frequency = <8000000>;
>> +       /* arbitrary range because all GPIOs are in software mode */
>> +       microchip,sgpio-port-ranges = <0 11>;
>> +       status = "okay";
>> +};
>> +
>> +&sgpio_in {
>> +       ngpios = <128>;
>> +};
>> +
>> +&sgpio_out {
>> +       ngpios = <128>;
>> +};
>> +
>> +&spi3 {
>> +       pinctrl-0 = <&fc3_b_pins>;
>> +       pinctrl-names = "default";
>> +       cs-gpios = <&gpio 46 GPIO_ACTIVE_LOW>;
>> +       status = "okay";
>> +};
>> +
>> +&watchdog {
>> +       status = "okay";
>> +};
>> --
>> 2.30.2
>> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ