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

["Yulin Lu" <luyulin@eswincomputing.com>]

> > +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