linux-phy.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Behún" <kabel@kernel.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: linux-phy@lists.infradead.org,
	Kishon Vijay Abraham I <kishon@ti.com>,
	pali@kernel.org, Miquel Raynal <miquel.raynal@bootlin.com>,
	Gregory CLEMENT <gregory.clement@bootlin.com>
Subject: Re: [PATCH phy v3 2/5] phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation
Date: Thu, 3 Feb 2022 22:39:13 +0100	[thread overview]
Message-ID: <20220203223913.41e9f375@thinkpad> (raw)
In-Reply-To: <YfqotCQ8TRWHp8Vz@matsya>

On Wed, 2 Feb 2022 21:22:20 +0530
Vinod Koul <vkoul@kernel.org> wrote:

> > +	if (ret) {
> > +		dev_err(lane->dev, "Failed to lock SATA PLL\n");
> > +		return ret;
> > +	}
> > +
> > +	return 0;  
> 
> This can be:
>         ret = comphy_lane_reg_poll(lane, COMPHY_DIG_LOOPBACK_EN, ...);
>         if (ret)
>                 dev_err(...);
> 
>         return ret;

Ok, will change all such occurances in v4.


> > +	/*
> > +	 * 12. As long as DFE function needs to be enabled in any mode,
> > +	 * COMPHY register DFE_UPDATE_EN[5:0] shall be programmed to 0x3F
> > +	 * for real chip during COMPHY power on.  
> 
> Okay where is this step..?

As is written in rest of the comment, the value of DFE_UPDATE_EN is
0x3F already (after PHY reset, because it is default value of the
register), so this is not needed. I have changed the comment a little in
v4, but I would like to leave it here.

> > +	 * The step 14 exists (and empty) in the original initialization flow
> > +	 * obtained from the verification team. According to the functional
> > +	 * specification DFE_UPDATE_EN already has the default value 0x3F
> > +	 */

snip

> > +
> > +	/*
> > +	 * 14. [Simulation Only] should not be used for real chip.
> > +	 * By pass power up calibration by programming EXT_FORCE_CAL_DONE
> > +	 * (R02h[9]) to 1 to shorten COMPHY simulation time.
> > +	 */
> > +
> > +	/*
> > +	 * 15. [Simulation Only: should not be used for real chip]
> > +	 * Program COMPHY register FAST_DFE_TIMER_EN=1 to shorten RX training
> > +	 * simulation time.
> > +	 */  
> 
> can we skip Simulation Only comments?

ok, removed in v4 and subsequent steps renumbered

> > +static void
> > +mvebu_a3700_comphy_usb3_power_off(struct mvebu_a3700_comphy_lane *lane)
> > +{
> > +	/*
> > +	 * Currently the USB3 MAC sets the USB3 PHY to low state, so we do not
> > +	 * need to power off USB3 PHY again.
> > +	 */
> > +}  
> 
> lets drop the dummy and move this comment to caller..?

ok

will send v4 in a couple of minutes.

Thanks, Vinod.

Marek

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2022-02-03 21:39 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-03 14:32 [PATCH phy v3 0/5] Armada 3720 comphy native implementation Marek Behún
2022-01-03 14:32 ` [PATCH phy v3 1/5] phy: marvell: phy-mvebu-a3700-comphy: Remove port from driver configuration Marek Behún
2022-01-03 14:32 ` [PATCH phy v3 2/5] phy: marvell: phy-mvebu-a3700-comphy: Add native kernel implementation Marek Behún
2022-02-02 15:52   ` Vinod Koul
2022-02-03 21:39     ` Marek Behún [this message]
2022-01-03 14:32 ` [PATCH phy v3 3/5] Revert "ata: ahci: mvebu: Make SATA PHY optional for Armada 3720" Marek Behún
2022-01-03 14:32 ` [PATCH phy v3 4/5] Revert "usb: host: xhci: mvebu: make USB 3.0 " Marek Behún
2022-01-03 14:32 ` [PATCH phy v3 5/5] Revert "PCI: aardvark: Fix initialization with old Marvell's Arm Trusted Firmware" Marek Behún
2022-01-17 14:58 ` [PATCH phy v3 0/5] Armada 3720 comphy native implementation Marek Behún
2022-01-27  5:27   ` Vinod Koul

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 \
    --in-reply-to=20220203223913.41e9f375@thinkpad \
    --to=kabel@kernel.org \
    --cc=gregory.clement@bootlin.com \
    --cc=kishon@ti.com \
    --cc=linux-phy@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=pali@kernel.org \
    --cc=vkoul@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).