RE: [PATCH net-next v4 00/12] Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface

Selvamani Rajagopal
2024-06-07T04:41:25Z

> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Thursday, June 6, 2024 6:12 AM
> To: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>
> Cc: Parthiban.Veerasooran@microchip.com; Piergiorgio Beruto
> <Pier.Beruto@onsemi.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> horms@kernel.org; saeedm@nvidia.com; anthony.l.nguyen@intel.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; corbet@lwn.net;
> linux-doc@vger.kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; Horatiu.Vultur@microchip.com;
> ruanjinjie@huawei.com; Steen.Hegelund@microchip.com;
> vladimir.oltean@nxp.com; UNGLinuxDriver@microchip.com;
> Thorsten.Kummermehr@microchip.com; Nicolas.Ferre@microchip.com;
> benjamin.bigler@bernformulastudent.ch; Viliam Vozar
> <Viliam.Vozar@onsemi.com>; Arndt Schuebel
> <Arndt.Schuebel@onsemi.com>
> Subject: Re: [PATCH net-next v4 00/12] Add support for OPEN Alliance
> 10BASE-T1x MACPHY Serial Interface
>
> [External Email]: This email arrived from an external source - Please
> exercise caution when opening any attachments or clicking on links.
>
> > I believe my client is configured to wrap at 70th characters.
> > Not sure why it is not doing it.
>
>
> It could be you also send a MIME obfuscated copy which is not wrapped
> correctly?
>
> > > > 1) Can we move memory map selector definitions
> > > (OA_TC6_PHY_C45_PCS_MMS2 and other 4 definitions) to the header
> > > file
> > > > include/linux/oa_tc6.h?
> > > > Also, if possible, could we add the MMS0, MMS1?. Our driver is
> > > using them. Of course, we could add it when we submit our driver.
> > >
> > > Interesting. So you have vendor registers outside of MMS 10-15?
> >
> > This is not about vendor registers. The current oa_tc6 defines
> > MMS selector values for 2, 3, 4, 5, 6. I am asking, if 0, 1 can be added,
> > which are meant for "Standard Control and Status" and MAC
> respectively,
> > according to MMS assignment table 6 on OA standard.
>
> But why would a MAC driver need access to those? Everything using
> those registers should be defined in the standard. So the framework
> should handle them.
>
> > One example I can think of is, to handle PHYINT status bit
> > that may be set in STATUS0 register. Another example could be,
> > to give a vendor flexibility to not to use interrupt mode.
>
> But that is part of the standard. Why would a driver need to do
> anything, the framework should handle PHYINT, calling
> phy_mac_interrupt(phydev).
>
> I really think you need to post patches. We can then discuss each use
> case, and i can give you concrete feedback.

True. That would be better. Will work on the patches so that it is clear.

>
> But in general, if it is part of the standard it should be in the
> framework. Support for features which are not part of the standard,
> and workarounds for where a device violates the standard, should be in
> the MAC driver, or the PHY driver.
>
> Andrew