linux-kernel - Re: Re: [PATCH v7 2/2] phy: eswin: Create eswin directory and add EIC7700 SATA PHY driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <133c22d5.2674.19bc5ff735f.Coremail.luyulin@eswincomputing.com>
Date: Fri, 16 Jan 2026 16:50:11 +0800 (GMT+08:00)
From: "Yulin Lu" <luyulin@...incomputing.com>
To: "Vinod Koul" <vkoul@...nel.org>
Cc: neil.armstrong@...aro.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, p.zabel@...gutronix.de,
	linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, ningyu@...incomputing.com,
	zhengyu@...incomputing.com, linmin@...incomputing.com,
	huangyifeng@...incomputing.com, fenglin@...incomputing.com,
	lianghujun@...incomputing.com
Subject: Re: Re: [PATCH v7 2/2] phy: eswin: Create eswin directory and add
 EIC7700 SATA PHY driver

> > +static int eic7700_sata_phy_init(struct phy *phy)
> > +{
> > +	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
> > +	u32 val;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(sata_phy->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	regmap_write(sata_phy->regmap, SATA_REF_CTRL1, SATA_CLK_RST_SOURCE_PHY);
> > +
> > +	val = FIELD_PREP(SATA_P0_PHY_TX_AMPLITUDE_GEN1_MASK, 0x42) |
> > +	      FIELD_PREP(SATA_P0_PHY_TX_AMPLITUDE_GEN2_MASK, 0x46) |
> > +	      FIELD_PREP(SATA_P0_PHY_TX_AMPLITUDE_GEN3_MASK, 0x73);
> > +	regmap_write(sata_phy->regmap, SATA_PHY_CTRL0, val);
> > +
> > +	val = FIELD_PREP(SATA_P0_PHY_TX_PREEMPH_GEN1_MASK, 0x5) |
> > +	      FIELD_PREP(SATA_P0_PHY_TX_PREEMPH_GEN2_MASK, 0x5) |
> > +	      FIELD_PREP(SATA_P0_PHY_TX_PREEMPH_GEN3_MASK, 0x8);
> 
> Where are the magic values you are writing coming from..?
> 

Hi Vinod,

These values set the TX preemphasis and amplitude parameters for the SATA PHY.
The actual numbers come from eye‑diagram tuning results on different hardware
development boards.
The current code reflects the settings for the Sifive HiFive Premier P550 board.
In the next patch I plan to move these into the devicetree (DTS).
Would that be acceptable?

> > +	regmap_write(sata_phy->regmap, SATA_PHY_CTRL1, val);
> > +
> > +	val = FIELD_PREP(SATA_LOS_LEVEL_MASK, 0x9) |
> > +	      FIELD_PREP(SATA_LOS_BIAS_MASK, 0x2);
> > +	regmap_write(sata_phy->regmap, SATA_LOS_IDEN, val);
> > +
> > +	val = SATA_M_CSYSREQ | SATA_S_CSYSREQ;
> > +	regmap_write(sata_phy->regmap, SATA_AXI_LP_CTRL, val);
> > +
> > +	val = SATA_REF_REPEATCLK_EN | SATA_REF_USE_PAD;
> > +	regmap_write(sata_phy->regmap, SATA_REF_CTRL, val);
> > +
> > +	val = FIELD_PREP(SATA_MPLL_MULTIPLIER_MASK, 0x3c);
> > +	regmap_write(sata_phy->regmap, SATA_MPLL_CTRL, val);
> > +
> > +	usleep_range(15, 20);
> > +
> > +	ret = reset_control_deassert(sata_phy->rst);
> > +	if (ret)
> > +		goto disable_clk;
> > +
> > +	ret = wait_for_phy_ready(sata_phy->regmap, SATA_P0_PHY_STAT,
> > +				 SATA_P0_PHY_READY, 1);
> > +	if (ret < 0) {
> > +		dev_err(&sata_phy->phy->dev, "PHY READY check failed\n");
> > +		goto disable_clk;
> > +	}
> > +
> > +	return 0;
> > +
> > +disable_clk:
> > +	clk_disable_unprepare(sata_phy->clk);
> > +	return ret;
> > +}
> > +
> > +static int eic7700_sata_phy_exit(struct phy *phy)
> > +{
> > +	struct eic7700_sata_phy *sata_phy = phy_get_drvdata(phy);
> > +	int ret;
> > +
> > +	ret = reset_control_assert(sata_phy->rst);
> > +	if (ret)
> > +		return ret;
> > +
> > +	clk_disable_unprepare(sata_phy->clk);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct phy_ops eic7700_sata_phy_ops = {
> > +	.init		= eic7700_sata_phy_init,
> > +	.exit		= eic7700_sata_phy_exit,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static int eic7700_sata_phy_probe(struct platform_device *pdev)
> > +{
> > +	struct eic7700_sata_phy *sata_phy;
> > +	struct phy_provider *phy_provider;
> > +	struct device *dev = &pdev->dev;
> > +	struct resource *res;
> > +	void __iomem *regs;
> > +
> > +	sata_phy = devm_kzalloc(dev, sizeof(*sata_phy), GFP_KERNEL);
> > +	if (!sata_phy)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res)
> > +		return -ENOENT;
> > +
> > +	regs = devm_ioremap(dev, res->start, resource_size(res));
> > +	if (IS_ERR(regs))
> > +		return PTR_ERR(regs);
> 
> devm_platform_get_and_ioremap_resource() please
> 

As explained in my “v6 → v5” changes in the cover‑letter:
“Map the I/O resource with platform_get_resource and devm_ioremap
instead of the devm_platform_ioremap_resource API,
because the address region of the SATA‑PHY falls into the region of
the HSP clock & reset that has already been obtained by the HSP
clock‑and‑reset driver.”
The HSP clock-and-reset driver uses devm_platform_get_and_ioremap_resource(),
meaning this region has already been requested.
The HSP clock-and-reset driver is also currently being upstreamed.

Best regards,
Yulin Lu