Re: [PATCH v7 08/15] ahci-imx: Port to library-ised ahci_platform

[Russell King - ARM Linux <linux@arm.linux.org.uk>]

On Sat, Feb 22, 2014 at 04:53:37PM +0100, Hans de Goede wrote:
> +		/*
> +		 * set PHY Paremeters, two steps to configure the GPR13,
> +		 * one write for rest of parameters, mask of first write
> +		 * is 0x07ffffff, and the other one write for setting
> +		 * the mpll_clk_en.
> +		 */
> +		regmap_update_bits(imxpriv->gpr, IOMUXC_GPR13,
> +				   IMX6Q_GPR13_SATA_RX_EQ_VAL_MASK |
> +				   IMX6Q_GPR13_SATA_RX_LOS_LVL_MASK |
> +				   IMX6Q_GPR13_SATA_RX_DPLL_MODE_MASK |
> +				   IMX6Q_GPR13_SATA_SPD_MODE_MASK |
> +				   IMX6Q_GPR13_SATA_MPLL_SS_EN |
> +				   IMX6Q_GPR13_SATA_TX_ATTEN_MASK |
> +				   IMX6Q_GPR13_SATA_TX_BOOST_MASK |
> +				   IMX6Q_GPR13_SATA_TX_LVL_MASK |
> +				   IMX6Q_GPR13_SATA_MPLL_CLK_EN |
> +				   IMX6Q_GPR13_SATA_TX_EDGE_RATE,
> +				   IMX6Q_GPR13_SATA_RX_EQ_VAL_3_0_DB |
> +				   IMX6Q_GPR13_SATA_RX_LOS_LVL_SATA2M |
> +				   IMX6Q_GPR13_SATA_RX_DPLL_MODE_2P_4F |
> +				   IMX6Q_GPR13_SATA_SPD_MODE_3P0G |
> +				   IMX6Q_GPR13_SATA_MPLL_SS_EN |
> +				   IMX6Q_GPR13_SATA_TX_ATTEN_9_16 |
> +				   IMX6Q_GPR13_SATA_TX_BOOST_3_33_DB |
> +				   IMX6Q_GPR13_SATA_TX_LVL_1_025_V);

While I see this, I'll stick an oar in.

It is totally wrong for this to be hard-coded into the driver - many
of these should be specified via DT, since they're board specific.
These are already different from the reset default in this register.

The side effect of this hard coding is that eSATA just doesn't work
at all on some boards - the board I have requires TX_LVL_1_104V and
TX_BOOST_0_00_DB.

Hence, I'd ask that while you move stuff like this around, you bear
in mind that we do need to add additional properties.

I'm in two minds about this - whether to make the existing binding
with its compatible string always use these settings, and invent a
new compatible string for one which is fully configurable (as it
_should_ have been from the very start) or whether to make this
the default if the various properties aren't specified.

Either way, this driver is electrically unusable as it stands here.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.