Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Fri, 8 Dec 2023 17:28:20 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Jiangfeng Ma <Jiangfeng.Ma@synopsys.com>
Cc:     Jiangfeng Ma <Jiangfeng.Ma@synopsys.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <Jose.Abreu@synopsys.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Simon Horman <horms@kernel.org>,
        Andrew Halaney <ahalaney@redhat.com>,
        Bartosz Golaszewski <bartosz.golaszewski@linaro.org>,
        Shenwei Wang <shenwei.wang@nxp.com>,
        Johannes Zink <j.zink@pengutronix.de>,
        "Russell King (Oracle" <rmk+kernel@armlinux.org.uk>,
        Jochen Henneberg <jh@henneberg-systemdesign.com>,
        "open list:STMMAC ETHERNET DRIVER" <netdev@vger.kernel.org>,
        "moderated list:ARM/STM32 ARCHITECTURE" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "moderated list:ARM/STM32 ARCHITECTURE" 
        <linux-arm-kernel@lists.infradead.org>,
        open list <linux-kernel@vger.kernel.org>,
        James Li <James.Li1@synopsys.com>,
        Martin McKenny <Martin.McKenny@synopsys.com>
Subject: Re: [PATCH] net:stmmac:stmmac_platform:Add snps,xpcs devicetree
 parsing
Message-ID: <uzss3af2cklc5bx5apszoegafeaaiv7o7iwgrgrml4grkyev6p@6tme4hdb4mkn>
References: <SA0PR12MB44138E48A245378CF54D2F9ED98AA@SA0PR12MB4413.namprd12.prod.outlook.com>
 <20231208091408.071680db@device.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231208091408.071680db@device.home>
Precedence: bulk

Hi Maxime, Jiangfeng

On Fri, Dec 08, 2023 at 09:14:08AM +0100, Maxime Chevallier wrote:
> Hello,
> 
> On Fri, 8 Dec 2023 07:02:19 +0000
> Jiangfeng Ma <Jiangfeng.Ma@synopsys.com> wrote:
> 
> > In order to setup xpcs, has_xpcs must be set to a non-zero value.
> > Add new optional devicetree properties representing this.
> > 
> > If has_xpcs is set to true, then xpcs_an_inband should preferably be
> > consistent with it, Otherwise, some errors may occur when starting
> > the network, For example, the phy interface that xpcs already supports,
> > but link up fails.
> 
> Can you elaborate on why you need this, and on which platform
> especially ? Usually drivers for the various stmmac variants know if
> they have XPCS or not, or can guess it based on some info such as the
> configured PHY mode (dwmac-intel).
> 
> Besides that, there are a few issues with your submission. If DT is the
> way to go (and I don't say it is), you would also need to update the
> bindings to document that property.
> 
> > The types of has_xpcs and xpcs_an_inband are unsigned int,
> > and generally used as flags. So it may be more reasonable to set them to
> > bool type. This can also be confirmed from the type of @ovr_an_inband.
> 
> And this part would go into a separate patch.

In addition to what Maxime already said having DT-bindings adjusted to
fit to the pattern implemented in the software part is a wrong way to
go. The best choice in this case is to add the DW XPCS DT-node to the
DW MAC MDIO/MI bus and then use the "pcs-handle" to inform the MAC
(mainly it's driver) of what PCS-device is actually attached to it.
The series I submitted on this week is exactly about that:
https://lore.kernel.org/netdev/20231205103559.9605-1-fancer.lancer@gmail.com/
I guess I'll need about a month or so to settle all the comments, but
the solution implemented there will be better than this one really.

-Serge(y)

> 
> Thanks,
> 
> Maxime
>