[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<BYAPR02MB5958A4D667D13071E023B18F83F52@BYAPR02MB5958.namprd02.prod.outlook.com>
Date: Fri, 24 May 2024 20:52:22 +0000
From: Selvamani Rajagopal <Selvamani.Rajagopal@...emi.com>
To: "Parthiban.Veerasooran@...rochip.com"
<Parthiban.Veerasooran@...rochip.com>,
"andrew@...n.ch" <andrew@...n.ch>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com"
<edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"horms@...nel.org"
<horms@...nel.org>,
"saeedm@...dia.com" <saeedm@...dia.com>,
"anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"corbet@....net" <corbet@....net>,
"linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>,
"Horatiu.Vultur@...rochip.com"
<Horatiu.Vultur@...rochip.com>,
"ruanjinjie@...wei.com"
<ruanjinjie@...wei.com>,
"Steen.Hegelund@...rochip.com"
<Steen.Hegelund@...rochip.com>,
"vladimir.oltean@....com"
<vladimir.oltean@....com>,
"UNGLinuxDriver@...rochip.com"
<UNGLinuxDriver@...rochip.com>,
"Thorsten.Kummermehr@...rochip.com"
<Thorsten.Kummermehr@...rochip.com>,
Piergiorgio Beruto
<Pier.Beruto@...emi.com>,
"Nicolas.Ferre@...rochip.com"
<Nicolas.Ferre@...rochip.com>,
"benjamin.bigler@...nformulastudent.ch"
<benjamin.bigler@...nformulastudent.ch>
Subject: RE: [PATCH net-next v4 00/12] Add support for OPEN Alliance
10BASE-T1x MACPHY Serial Interface
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@...rochip.com
> <Parthiban.Veerasooran@...rochip.com>
> Sent: Friday, May 10, 2024 4:22 AM
> To: andrew@...n.ch
> Cc: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; horms@...nel.org; saeedm@...dia.com;
> anthony.l.nguyen@...el.com; netdev@...r.kernel.org; linux-
> kernel@...r.kernel.org; corbet@....net; linux-doc@...r.kernel.org;
> robh+dt@...nel.org; krzysztof.kozlowski+dt@...aro.org;
> conor+dt@...nel.org; devicetree@...r.kernel.org;
> Horatiu.Vultur@...rochip.com; ruanjinjie@...wei.com;
> Steen.Hegelund@...rochip.com; vladimir.oltean@....com;
> UNGLinuxDriver@...rochip.com;
> Thorsten.Kummermehr@...rochip.com; Piergiorgio Beruto
> <Pier.Beruto@...emi.com>; Selvamani Rajagopal
> <Selvamani.Rajagopal@...emi.com>; Nicolas.Ferre@...rochip.com;
> benjamin.bigler@...nformulastudent.ch
> 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@...rochip.com 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
> >
Powered by blists - more mailing lists