Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201203214645.31217-1-TheSven73@gmail.com> <20201204152456.247769b1@kicinski-fedora-pc1c0hjn.DHCP.thefacebook.com>
 <20201204234321.GJ2400258@lunn.ch>
In-Reply-To: <20201204234321.GJ2400258@lunn.ch>
From:   Sven Van Asbroeck <thesven73@gmail.com>
Date:   Sat, 5 Dec 2020 09:35:10 -0500
Message-ID: <CAGngYiXYLnJUMrPqiJtRqFxF6xo01FYBvmn1Rq-wbjBDOuC-9Q@mail.gmail.com>
Subject: Re: [PATCH net v1] net: dsa: ksz8795: use correct number of physical ports
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     Jakub Kicinski <kuba@kernel.org>,
        Woojung Huh <woojung.huh@microchip.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        David S Miller <davem@davemloft.net>,
        Marek Vasut <marex@denx.de>,
        Tristram Ha <Tristram.Ha@microchip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
        netdev <netdev@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Jakub, Andrew,

On Fri, Dec 4, 2020 at 6:24 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> All the port counts here are -1 compared to datasheets, so I'm assuming
> the are not supposed to include the host facing port or something?
>
> Can you describe the exact problem you're trying to solve?
>

The ksz8795 driver refuses to accept my devicetree (see below). It doesn't like
the presence of the cpu node. But I really need a cpu node, because phy-mode is
crucially important to my application: without phy-mode = "rgmii-id", the
ksz8795's ingress bit does not get set, and communication fails.

I was already suspicious of my fix: why would such an important chip property
be wrong? But changing it was very seductive: after all, the ksz8795's headline
is "Integrated 5-Port 10/100-Managed Ethernet Switch", and the driver code says
this:

        .port_cnt = 4,          /* total physical port count */

I can see now that this should be fixed more generally. The of parsing code
is using the wrong port count variable. I'll submit that shortly.

That said, when I look at this driver, I get very confused between port_cnt,
num_ports, and whether they include or exclude the cpu port. Until this gets
cleaned up, maybe the comment above can be improved, so developers don't get
too confused at least? I'll submit a patch for that too.

ethernet-switch@0 {
        compatible = "microchip,ksz8795";
        spi-max-frequency = <1000000>;
        reg = <0>;

        ports {
                #address-cells = <1>;
                #size-cells = <0>;
                port@0 {
                        reg = <0>;
                        label = "lan1";
                };
                port@1 {
                        reg = <1>;
                        label = "lan2";
                };
                port@2 {
                        reg = <2>;
                        label = "lan3";
                };
                port@3 {
                        reg = <3>;
                        label = "lan4";
                };
                port@4 {
                        /* driver errors out because
                         * reg >= dev->port_cnt (4)
                         */
                        reg = <4>;
                        label = "cpu";
                        ethernet = <&something>;
                        phy-mode = "rgmii-id";
                        fixed-link {
                                speed = <1000>;
                                full-duplex;
                        };
                };
        };
};