Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Oct 2022 07:54:49 +0200
From:   Frank Wunderlich <frank-w@public-files.de>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>,
        Frank Wunderlich <linux@fw-web.de>
CC:     linux-mediatek@lists.infradead.org,
        Alexander Couzens <lynxis@fe80.eu>,
        Felix Fietkau <nbd@nbd.name>, John Crispin <john@phrozen.org>,
        Sean Wang <sean.wang@mediatek.com>,
        Mark Lee <Mark-MC.Lee@mediatek.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>,
        netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: mtk_sgmii: implement mtk_pcs_ops
User-Agent: K-9 Mail for Android
Reply-to: frank-w@public-files.de
In-Reply-To: <Y07Wpd1A1xxLhIVc@shell.armlinux.org.uk>
References: <20221018153506.60944-1-linux@fw-web.de> <Y07Wpd1A1xxLhIVc@shell.armlinux.org.uk>
Message-ID: <949F5EE5-B22D-40E2-9783-0F75ACFE2C1F@public-files.de>
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Am 18=2E Oktober 2022 18:39:01 MESZ schrieb "Russell King (Oracle)" <linux@=
armlinux=2Eorg=2Euk>:
>Hi,
>
>A couple of points:
>
>On Tue, Oct 18, 2022 at 05:35:06PM +0200, Frank Wunderlich wrote:

>> +	regmap_read(mpcs->regmap, SGMSYS_PCS_CONTROL_1, &val);
>> +	state->an_complete =3D !!(val & SGMII_AN_COMPLETE);
>> +	state->link =3D !!(val & SGMII_LINK_STATYS);
>> +	state->pause =3D 0;
>
>Finally, something approaching a reasonable implementation for this!
>Two points however:
>1) There's no need to set state->pause if there is no way to get that
>   state=2E
>2) There should also be a setting for state->pause=2E

Currently it looks like pause cannot be controlled in sgmii-mode so we dis=
abled it here to not leave it undefined=2E Should i drop assignment here?

>> +}
>> +
>>  static const struct phylink_pcs_ops mtk_pcs_ops =3D {
>>  	=2Epcs_config =3D mtk_pcs_config,
>>  	=2Epcs_an_restart =3D mtk_pcs_restart_an,
>>  	=2Epcs_link_up =3D mtk_pcs_link_up,
>> +	=2Epcs_get_state =3D mtk_pcs_get_state,
>
>Please keep this in order - pcs_get_state should be just before
>pcs_config=2E

Ok,will change order

regards Frank