Re: [RESEND PATCH v2 7/9] mfd: rtsx: add support for rts524A

[Lee Jones <lee.jones@linaro.org>]

On Wed, 25 Feb 2015, 敬锐 wrote:

> 
> On 02/16/2015 10:28 PM, Lee Jones wrote:
> >
> > +static int rts524a_optimize_phy(struct rtsx_pcr *pcr)
> > +{
> > +	int err;
> > +
> > +	err = rtsx_pci_write_register(pcr, RTS524A_PM_CTRL3,
> > +		D3_DELINK_MODE_EN, 0x00);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	rtsx_pci_write_phy_register(pcr, PHY_PCR,
> > +		PHY_PCR_FORCE_CODE | PHY_PCR_OOBS_CALI_50 |
> > +		PHY_PCR_OOBS_VCM_08 | PHY_PCR_OOBS_SEN_90 | PHY_PCR_RSSI_EN);
> > +	rtsx_pci_write_phy_register(pcr, PHY_SSCCR3,
> > +		PHY_SSCCR3_STEP_IN | PHY_SSCCR3_CHECK_DELAY);
> > +
> > +	if (is_version(pcr, 0x524A, IC_VER_A)) {
> > +		rtsx_pci_write_phy_register(pcr, PHY_SSCCR3,
> > +			PHY_SSCCR3_STEP_IN | PHY_SSCCR3_CHECK_DELAY);
> > +		rtsx_pci_write_phy_register(pcr, PHY_SSCCR2,
> > +			PHY_SSCCR2_PLL_NCODE | PHY_SSCCR2_TIME0 |
> > +			PHY_SSCCR2_TIME2_WIDTH);
> > +		rtsx_pci_write_phy_register(pcr, PHY_ANA1A,
> > +			PHY_ANA1A_TXR_LOOPBACK | PHY_ANA1A_RXT_BIST |
> > +			PHY_ANA1A_TXR_BIST | PHY_ANA1A_REV);
> > +		rtsx_pci_write_phy_register(pcr, PHY_ANA1D,
> > +			PHY_ANA1D_DEBUG_ADDR);
> > +		rtsx_pci_write_phy_register(pcr, PHY_DIG1E,
> > +			PHY_DIG1E_REV | PHY_DIG1E_D0_X_D1 |
> > +			PHY_DIG1E_RX_ON_HOST | PHY_DIG1E_RCLK_REF_HOST |
> > +			PHY_DIG1E_RCLK_TX_EN_KEEP |
> > +			PHY_DIG1E_RCLK_TX_TERM_KEEP |
> > +			PHY_DIG1E_RCLK_RX_EIDLE_ON | PHY_DIG1E_TX_TERM_KEEP |
> > +			PHY_DIG1E_RX_TERM_KEEP | PHY_DIG1E_TX_EN_KEEP |
> > +			PHY_DIG1E_RX_EN_KEEP);
> > +	}
> > +
> > +	rtsx_pci_write_phy_register(pcr, PHY_ANA08,
> > +		PHY_ANA08_RX_EQ_DCGAIN | PHY_ANA08_SEL_RX_EN |
> > +		PHY_ANA08_RX_EQ_VAL | PHY_ANA08_SCP | PHY_ANA08_SEL_IPI);
> > To the uninitiated this function is mostly randomness.  How about some
> > nice comments to illuminate?
> I'm not clear with these setting either, it is used to fix some phy 
> setting, the default phy setting
> it not stable on some special platform, so we have to modify them by driver,
> newer version of chip will change its default value to more stable 
> configure, so some value is
> no need to setting for Version B/C...

That doesn't help me in any way.  Use the datasheet, look-up the
values and insert a nice, succinct explanation of what you're doing
and why it's required please.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog