lkml.org 
[lkml]   [2020]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 2/3] net: sparx5: Add Sparx5 switchdev driver
On 28.11.2020 22:28, Russell King - ARM Linux admin wrote:
>EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>On Sat, Nov 28, 2020 at 08:06:16PM +0100, Andrew Lunn wrote:
>> > +static void sparx5_phylink_mac_config(struct phylink_config *config,
>> > + unsigned int mode,
>> > + const struct phylink_link_state *state)
>> > +{
>> > + struct sparx5_port *port = netdev_priv(to_net_dev(config->dev));
>> > + struct sparx5_port_config conf;
>> > + int err = 0;
>> > +
>> > + conf = port->conf;
>> > + conf.autoneg = state->an_enabled;
>> > + conf.pause = state->pause;
>> > + conf.duplex = state->duplex;
>> > + conf.power_down = false;
>> > + conf.portmode = state->interface;
>> > +
>> > + if (state->speed == SPEED_UNKNOWN) {
>> > + /* When a SFP is plugged in we use capabilities to
>> > + * default to the highest supported speed
>> > + */
>>
>> This looks suspicious.
>
>Yes, it looks highly suspicious. The fact that
>sparx5_phylink_mac_link_up() is empty, and sparx5_phylink_mac_config()
>does all the work suggests that this was developed before the phylink
>re-organisation, and this code hasn't been updated for it.

Hi Russell,

The work started on 5.4 and I think I may have not really
understood the finer details in the changes in 5.9.
>
>Any new code for the kernel really ought to be updated for the new
>phylink methodology before it is accepted.
>

Could you point me to a good example of the new methodology?

>Looking at sparx5_port_config(), it also seems to use
>PHY_INTERFACE_MODE_1000BASEX for both 1000BASE-X and 2500BASE-X. All
>very well for the driver to do that internally, but it's confusing
>when it comes to reviewing this stuff, especially when people outside
>of the driver (such as myself) reviewing it need to understand what's
>going on with the configuration.

Hmmm. I better check if this is as intended.

>
>--
>RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
>FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Thanks for your comments.

BR
Steen

---------------------------------------
Steen Hegelund
steen.hegelund@microchip.com

\
 
 \ /
  Last update: 2020-11-30 15:15    [W:0.197 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site