Date:   Mon, 6 Mar 2023 22:19:05 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     =?utf-8?B?QXLEsW7DpyDDnE5BTA==?= <arinc.unal@arinc9.com>
Cc:     Sean Wang <sean.wang@mediatek.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        DENG Qingfang <dqfext@gmail.com>, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Russell King <linux@armlinux.org.uk>,
        Russell King <rmk+kernel@armlinux.org.uk>,
        =?utf-8?B?UmVuw6k=?= van Dorst <opensource@vdorst.com>,
        Alexander Couzens <lynxis@fe80.eu>,
        Ilya Lipnitskiy <ilya.lipnitskiy@gmail.com>,
        Richard van Schagen <richard@routerhints.com>,
        Frank Wunderlich <frank-w@public-files.de>,
        erkin.bozoglu@xeront.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Subject: Re: [RFC PATCH net] net: dsa: mt7530: move PLL setup out of port 6
 pad configuration
Message-ID: <20230306201905.yothcuxokzlk3mcq@skbuf>
References: <20230304125453.53476-1-arinc.unal@arinc9.com>
 <20230304125453.53476-1-arinc.unal@arinc9.com>
 <20230306154552.26o6sbwf3rfekcyz@skbuf>
 <65f84ef3-8f72-d823-e6f9-44d33a953697@arinc9.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <65f84ef3-8f72-d823-e6f9-44d33a953697@arinc9.com>
Precedence: bulk

On Mon, Mar 06, 2023 at 08:03:54PM +0300, Arınç ÜNAL wrote:
> Looking at the Wikipedia page for Media-independent interface [0], the data
> interface must be clocked at 125 MHz for gigabit MIIs, which I believe what
> the "PLL" here refers to. trgmii needs higher frequency in some cases so if
> both CPU ports are enabled, the table would be:
> 
>     priv->p5_interface        priv->p6_interface       ncpo1 value
>         gmii                     rgmii                     125MHz
>         mii                      rgmii                     125MHz
>         rgmii                    rgmii                     125MHz
>         gmii                     trgmii                    125-250MHz
>         mii                      trgmii                    125-250MHz
>         rgmii                    trgmii                    125-250MHz
> 
> [0] https://en.wikipedia.org/wiki/Media-independent_interface#GMII

Wikipedia will only tell you what the frequency of the interface signals
needs to be. That is useful to keep in mind, but without information from
the datasheet regarding what the SoC's clock distribution tree looks like,
it's hard to know how that interface clock is derived from internal PLLs
and ultimately from the oscillators. I was hoping that was the kind of
information you could provide. The manuals I have access to, through charity,
don't say anything on that front.

Since I don't know what I'm commenting on, I'll stop commenting any further.

> > right now, you let the p6_interface logic overwrite the ncpo1 selected
> > by the p5_interface logic like crazy, and it's not clear to me that this
> > is what you want.
> 
> This seems to be fine as p6 sets the frequency either the same or higher.

(...)

> This looks much better, thanks a lot! The only missing part is setting the
> PLL frequency when only port 5 is enabled.

True. Although with the limited information I have, I'm not sure that
the ncpo1 value written into CORE_PLL_GROUP5 is needed by port5 either
way. The fact that you claim port5 works when ncpo1 ranges from 125 to
250 MHz tells me that it's either very tolerant of the ncpo1 value
(through mechanisms unknown to me), or simply unaffected by it (more
likely ATM). Since I don't have any details regarding the value, I'd
just like to treat the configuration procedure as plain code, and not
make any changes until there's a proof that they're needed.

> I'll test it regardless.

Thanks.