From: Florian Fainelli <email@example.com> To: Dan Murphy <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, email@example.com Cc: firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com Subject: Re: [PATCH net-master 1/1] net: phy: dp83867: Add speed optimization feature Date: Fri, 31 Jan 2020 10:42:05 -0800 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On 1/31/20 10:29 AM, Dan Murphy wrote: > Florian > > On 1/31/20 11:49 AM, Florian Fainelli wrote: >> On 1/31/20 7:11 AM, Dan Murphy wrote: >>> Set the speed optimization bit on the DP83867 PHY. >>> This feature can also be strapped on the 64 pin PHY devices >>> but the 48 pin devices do not have the strap pin available to enable >>> this feature in the hardware. PHY team suggests to have this bit set. >> OK, but why and how does that optimization work exactly? > > I described this in the cover letter. And it is explained in the data > sheet Section 18.104.22.168 Sorry I complete missed that and just focused on the patch, you should consider not providing a cover letter for a single patch, and especially not when the cover letter contains more information than the patch commit message itself. > >> Departing from >> the BMSR reads means you possibly are going to introduce bugs and/or >> incomplete information. For instance, you set phydev->pause and >> phydev->asym_pause to 0 now, is there no way to extract what the link >> partner has advertised? > > I was using the marvel.c as my template as it appears to have a separate > status register as well. > > Instead of setting those bits in the call back I can call the > genphy_read_status then override the duplex and speed based on the > physts register like below. This way link status and pause values can > be updated and then we can update the speed and duplex settings. > > ret = genphy_read_status(phydev); > if (ret) > return ret; > > if (status < 0) > return status; > > if (status & DP83867_PHYSTS_DUPLEX) > phydev->duplex = DUPLEX_FULL; > else > phydev->duplex = DUPLEX_HALF; > > if (status & DP83867_PHYSTS_1000) > phydev->speed = SPEED_1000; > else if (status & DP83867_PHYSTS_100) > phydev->speed = SPEED_100; > else > phydev->speed = SPEED_10; > OK, but what if they disagree, are they consistently latched with respect to one another? > >>> With this bit set the PHY will auto negotiate and report the link >>> parameters in the PHYSTS register and not in the BMCR. >> That should be BMSR, the BMCR is about control, not status. > > OK. > > Dan > -- Florian
next prev parent reply other threads:[~2020-01-31 18:42 UTC|newest] Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-31 15:11 [PATCH net-master 0/1] DP83867 Speed " Dan Murphy 2020-01-31 15:11 ` [PATCH net-master 1/1] net: phy: dp83867: Add speed " Dan Murphy 2020-01-31 17:10 ` Jakub Kicinski 2020-01-31 17:14 ` Dan Murphy 2020-01-31 18:11 ` Jakub Kicinski 2020-01-31 18:36 ` Dan Murphy 2020-01-31 17:49 ` Florian Fainelli 2020-01-31 18:29 ` Dan Murphy 2020-01-31 18:42 ` Florian Fainelli [this message] 2020-01-31 19:14 ` Dan Murphy 2020-01-31 19:29 ` Florian Fainelli 2020-01-31 19:54 ` Dan Murphy 2020-01-31 20:02 ` Florian Fainelli 2020-01-31 20:10 ` Dan Murphy 2020-01-31 20:56 ` Heiner Kallweit 2020-02-01 1:30 ` Dan Murphy 2020-02-01 4:57 ` Florian Fainelli
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH net-master 1/1] net: phy: dp83867: Add speed optimization feature' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).