Andrew/Parthiban, Sorry I couldn't get back earlier on this subject as I just had to time review the organization of the code with respect to our device. I have a proposal for the way MDIO APIs are implemented in common code (oa_tc6.c). At present, mii_bus structure is in common code without any visibility to vendor specific code. Though this is a good design to get consistent behavior between different vendors, we can't customize the functions stored in read, write, read_c45, and write_c45 function pointers. In our MDIO functions, we do certain things based on PHY ID, also our driver deal with vendor specific register, MMS 12 (refer Table 6 in section 9.1 Open Alliance MAC-PHY Serial interface specification document.) I hope it is not uncommon to expect the ability to implement vendor specific mdio read/write. At present, there is no alternative to functions like oa_tc6_mdiobus_read, oa_tc6_mdiobus_write, oa_tc6_mdiobus_read_c45, and oa_tc6_mdiobus_write_c45. I am sure we can resolve this issue in many ways. One way is to provide a public function to populate mii_bus pointer (tc6->mdiobus in the code) from the vendor driver, or provide a way the pass when we call oa_tc6_init. Vendors who don't require such customization can use existing functionality. Those who require customization, can have extra code before calling this standard MDIO functions. Sincerely Selva > -----Original Message----- > From: Parthiban.Veerasooran@xxxxxxxxxxxxx > <Parthiban.Veerasooran@xxxxxxxxxxxxx> > Sent: Friday, May 10, 2024 4:22 AM > To: andrew@xxxxxxx > Cc: davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; > pabeni@xxxxxxxxxx; horms@xxxxxxxxxx; saeedm@xxxxxxxxxx; > anthony.l.nguyen@xxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; corbet@xxxxxxx; linux-doc@xxxxxxxxxxxxxxx; > robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; > conor+dt@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > Horatiu.Vultur@xxxxxxxxxxxxx; ruanjinjie@xxxxxxxxxx; > Steen.Hegelund@xxxxxxxxxxxxx; vladimir.oltean@xxxxxxx; > UNGLinuxDriver@xxxxxxxxxxxxx; > Thorsten.Kummermehr@xxxxxxxxxxxxx; Piergiorgio Beruto > <Pier.Beruto@xxxxxxxxxx>; Selvamani Rajagopal > <Selvamani.Rajagopal@xxxxxxxxxx>; Nicolas.Ferre@xxxxxxxxxxxxx; > benjamin.bigler@xxxxxxxxxxxxxxxxxxxxx > 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. > > Hi Andrew, > > On 10/05/24 2:09 am, Andrew Lunn wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > > > On Thu, May 09, 2024 at 01:04:52PM +0000, > Parthiban.Veerasooran@xxxxxxxxxxxxx wrote: > >> Hi Andrew, > >> > >> On 08/05/24 10:34 pm, Andrew Lunn wrote: > >>> EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > >>> > >>>> Yes. I tried this test. It works as expected. > >>> > >>>> Each LAN8651 received approximately 3Mbps with lot of > "Receive buffer > >>>> overflow error". I think it is expected as the single SPI master has to > >>>> serve both LAN8651 at the same time and both LAN8651 will be > receiving > >>>> 10Mbps on each. > >>> > >>> Thanks for testing this. > >>> > >>> This also shows the "Receive buffer overflow error" needs to go > away. > >>> Either we don't care at all, and should not enable the interrupt, or > >>> we do care and should increment a counter. > >> Thanks for your comments. I think, I would go for your 2nd proposal > >> because having "Receive buffer overflow error" enabled will indicate > the > >> cause of the poor performance. > >> > >> Already we have, > >> tc6->netdev->stats.rx_dropped++; > >> to increment the rx dropped counter in case of receive buffer > overflow. > >> > >> May be we can remove the print, > >> net_err_ratelimited("%s: Receive buffer overflow error\n", > >> tc6->netdev->name); > >> as it might lead to additional poor performance by adding some > delay. > >> > >> Could you please provide your opinion on this? > > > > This is your code. Ideally you should decide. I will only add review > > comments if i think it is wrong. Any can decide between any correct > > option. > Sure, thanks for your advice. Let me stick with the above proposal until > I get any others opinion. > > Best regards, > Parthiban V > > > > Andrew > >