diff mbox

[v4,1/7] phy: add a driver for the Berlin SATA PHY

Message ID 1400576675-25265-2-git-send-email-antoine.tenart@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Antoine Tenart May 20, 2014, 9:04 a.m. UTC 
The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.

The mode selection can let us think this PHY can be configured to fit
other purposes. But there are reasons to think the SATA mode will be
the only one usable: the PHY registers are only accessible indirectly
through two registers in the SATA range, the PHY seems to be integrated
and no information tells us the contrary. For these reasons, make the
driver a SATA PHY driver.

Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
---
 drivers/phy/Kconfig           |   5 +
 drivers/phy/Makefile          |   1 +
 drivers/phy/phy-berlin-sata.c | 230 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/phy/phy-berlin-sata.c

Comments

Sebastian Hesselbarth May 20, 2014, 9:11 a.m. UTC | #1 
On 05/20/2014 11:04 AM, Antoine Ténart wrote:
> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
>
> The mode selection can let us think this PHY can be configured to fit
> other purposes. But there are reasons to think the SATA mode will be
> the only one usable: the PHY registers are only accessible indirectly
> through two registers in the SATA range, the PHY seems to be integrated
> and no information tells us the contrary. For these reasons, make the
> driver a SATA PHY driver.
>
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
[...]
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> new file mode 100644
> index 000000000000..597f008cae32
> --- /dev/null
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -0,0 +1,230 @@
> +/*
> + * Marvell Berlin SATA PHY driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +#define HOST_VSA_ADDR		0x0
> +#define HOST_VSA_DATA		0x4
> +#define PORT_VSR_ADDR           0x78
> +#define PORT_VSR_DATA           0x7c

Above two lines are indented with spaces.

> +#define PORT_SCR_CTL		0x2c
> +
> +#define CONTROL_REGISTER	0x0
> +#define MBUS_SIZE_CONTROL	0x4
> +
> +#define POWER_DOWN_PHY0			BIT(6)
> +#define POWER_DOWN_PHY1			BIT(14)
> +#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
> +#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
> +
> +#define PHY_BASE                0x200

ditto.

> +
> +/* register 0x01 */
> +#define REF_FREF_SEL_25         BIT(0)
> +#define PHY_MODE_SATA           (0x0 << 5)

ditto.

> +
> +/* register 0x02 */
> +#define USE_MAX_PLL_RATE        BIT(12)

ditto.

> +
> +/* register 0x23 */
> +#define DATA_BIT_WIDTH_10       (0x0 << 10)
> +#define DATA_BIT_WIDTH_20       (0x1 << 10)
> +#define DATA_BIT_WIDTH_40       (0x2 << 10)

ditto.

> +
> +/* register 0x25 */
> +#define PHY_GEN_MAX_1_5         (0x0 << 10)
> +#define PHY_GEN_MAX_3_0         (0x1 << 10)
> +#define PHY_GEN_MAX_6_0         (0x2 << 10)

ditto.

FWIW,

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

> +
> +#define BERLIN_SATA_PHY_NB	2
> +
> +#define to_berlin_sata_phy_priv(desc)	\
> +	container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
> +
> +struct phy_berlin_desc {
> +	struct phy	*phy;
> +	u32		val;
> +	unsigned	index;
> +};
> +
> +struct phy_berlin_priv {
> +	void __iomem		*base;
> +	spinlock_t		lock;
> +	struct phy_berlin_desc	phys[BERLIN_SATA_PHY_NB];
> +};
> +
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
> +					       u32 mask, u32 val)
> +{
> +	u32 regval;
> +
> +	/* select register */
> +	writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> +
> +	/* set bits */
> +	regval = readl(ctrl_reg + PORT_VSR_DATA);
> +	regval &= ~mask;
> +	regval |= val;
> +	writel(regval, ctrl_reg + PORT_VSR_DATA);
> +}
> +
> +static int phy_berlin_sata_power_on(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> +	void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
> +	int ret = 0;
> +	u32 regval;
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power on PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval &= ~(desc->val);
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* Configure MBus */
> +	writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* set PHY mode and ref freq to 25 MHz */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> +				    REF_FREF_SEL_25 | PHY_MODE_SATA);
> +
> +	/* set PHY up to 6 Gbps */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> +
> +	/* set 40 bits width */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
> +
> +	/* use max pll rate */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> +
> +	/* set the controller speed */
> +	writel(0x31, ctrl_reg + PORT_SCR_CTL);
> +
> +	spin_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int phy_berlin_sata_power_off(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> +	u32 regval;
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power down PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= desc->val;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	spin_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> +					     struct of_phandle_args *args)
> +{
> +	struct phy_berlin_priv *priv = dev_get_drvdata(dev);
> +
> +	if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
> +		return ERR_PTR(-ENODEV);
> +
> +	return priv->phys[args->args[0]].phy;
> +}
> +
> +static struct phy_ops phy_berlin_sata_ops = {
> +	.power_on	= phy_berlin_sata_power_on,
> +	.power_off	= phy_berlin_sata_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy_berlin_desc desc[] = {
> +	{ .val = POWER_DOWN_PHY0 },
> +	{ .val = POWER_DOWN_PHY1 },
> +	{ },
> +};
> +
> +static int phy_berlin_sata_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +	struct phy_berlin_priv *priv;
> +	struct resource *res;
> +	int i;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	dev_set_drvdata(dev, priv);
> +	spin_lock_init(&priv->lock);
> +
> +	for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
> +		phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
> +		if (IS_ERR(phy)) {
> +			dev_err(dev, "failed to create PHY %d\n", i);
> +			return PTR_ERR(phy);
> +		}
> +
> +		priv->phys[i].phy = phy;
> +		priv->phys[i].val = desc[i].val;
> +		priv->phys[i].index = i;
> +		phy_set_drvdata(phy, &priv->phys[i]);
> +
> +		/* Make sure the PHY is off */
> +		phy_berlin_sata_power_off(phy);
> +	}
> +
> +	phy_provider =
> +		devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> +	{ .compatible = "marvell,berlin-sata-phy" },
> +	{ },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> +	.probe	= phy_berlin_sata_probe,
> +	.driver	= {
> +		.name		= "phy-berlin-sata",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= phy_berlin_sata_of_match,
> +	},
> +};
> +module_platform_driver(phy_berlin_sata_driver);
> +
> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
> +MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
> +MODULE_LICENSE("GPL v2");
>
Antoine Tenart May 20, 2014, 9:15 a.m. UTC | #2 
On Tue, May 20, 2014 at 11:11:17AM +0200, Sebastian Hesselbarth wrote:
> On 05/20/2014 11:04 AM, Antoine Ténart wrote:
> >+#define HOST_VSA_ADDR		0x0
> >+#define HOST_VSA_DATA		0x4
> >+#define PORT_VSR_ADDR           0x78
> >+#define PORT_VSR_DATA           0x7c
> 
> Above two lines are indented with spaces.

Indeed ... sorry for that.

> >+#define PORT_SCR_CTL		0x2c
> >+
> >+#define CONTROL_REGISTER	0x0
> >+#define MBUS_SIZE_CONTROL	0x4
> >+
> >+#define POWER_DOWN_PHY0			BIT(6)
> >+#define POWER_DOWN_PHY1			BIT(14)
> >+#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
> >+#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
> >+
> >+#define PHY_BASE                0x200
> 
> ditto.
> 
> >+
> >+/* register 0x01 */
> >+#define REF_FREF_SEL_25         BIT(0)
> >+#define PHY_MODE_SATA           (0x0 << 5)
> 
> ditto.
> 
> >+
> >+/* register 0x02 */
> >+#define USE_MAX_PLL_RATE        BIT(12)
> 
> ditto.
> 
> >+
> >+/* register 0x23 */
> >+#define DATA_BIT_WIDTH_10       (0x0 << 10)
> >+#define DATA_BIT_WIDTH_20       (0x1 << 10)
> >+#define DATA_BIT_WIDTH_40       (0x2 << 10)
> 
> ditto.
> 
> >+
> >+/* register 0x25 */
> >+#define PHY_GEN_MAX_1_5         (0x0 << 10)
> >+#define PHY_GEN_MAX_3_0         (0x1 << 10)
> >+#define PHY_GEN_MAX_6_0         (0x2 << 10)
> 
> ditto.

Antoine
Bartlomiej Zolnierkiewicz May 20, 2014, 12:34 p.m. UTC | #3 
Hi,

Few minor issues below..

On Tuesday, May 20, 2014 11:04:29 AM Antoine Ténart wrote:
> The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> 
> The mode selection can let us think this PHY can be configured to fit
> other purposes. But there are reasons to think the SATA mode will be
> the only one usable: the PHY registers are only accessible indirectly
> through two registers in the SATA range, the PHY seems to be integrated
> and no information tells us the contrary. For these reasons, make the
> driver a SATA PHY driver.
> 
> Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> ---
>  drivers/phy/Kconfig           |   5 +
>  drivers/phy/Makefile          |   1 +
>  drivers/phy/phy-berlin-sata.c | 230 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 236 insertions(+)
>  create mode 100644 drivers/phy/phy-berlin-sata.c
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 4906c27fa3bd..b31b1986fda4 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -15,6 +15,11 @@ config GENERIC_PHY
>  	  phy users can obtain reference to the PHY. All the users of this
>  	  framework should select this config.
>  
> +config PHY_BERLIN_SATA
> +	bool

Is there any real reason why this cannot be tristate?

> +	depends on ARCH_BERLIN && OF
> +	select GENERIC_PHY
> +
>  config PHY_EXYNOS_MIPI_VIDEO
>  	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
>  	depends on HAS_IOMEM
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index 7728518572a4..40278706ac1b 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
> +obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
>  obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
>  obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
>  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
> diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
> new file mode 100644
> index 000000000000..597f008cae32
> --- /dev/null
> +++ b/drivers/phy/phy-berlin-sata.c
> @@ -0,0 +1,230 @@
> +/*
> + * Marvell Berlin SATA PHY driver
> + *
> + * Copyright (C) 2014 Marvell Technology Group Ltd.
> + *
> + * Antoine Ténart <antoine.tenart@free-electrons.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/phy/phy.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +
> +#define HOST_VSA_ADDR		0x0
> +#define HOST_VSA_DATA		0x4
> +#define PORT_VSR_ADDR           0x78
> +#define PORT_VSR_DATA           0x7c
> +#define PORT_SCR_CTL		0x2c
> +
> +#define CONTROL_REGISTER	0x0
> +#define MBUS_SIZE_CONTROL	0x4
> +
> +#define POWER_DOWN_PHY0			BIT(6)
> +#define POWER_DOWN_PHY1			BIT(14)
> +#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
> +#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
> +
> +#define PHY_BASE                0x200
> +
> +/* register 0x01 */
> +#define REF_FREF_SEL_25         BIT(0)
> +#define PHY_MODE_SATA           (0x0 << 5)
> +
> +/* register 0x02 */
> +#define USE_MAX_PLL_RATE        BIT(12)
> +
> +/* register 0x23 */
> +#define DATA_BIT_WIDTH_10       (0x0 << 10)
> +#define DATA_BIT_WIDTH_20       (0x1 << 10)
> +#define DATA_BIT_WIDTH_40       (0x2 << 10)
> +
> +/* register 0x25 */
> +#define PHY_GEN_MAX_1_5         (0x0 << 10)
> +#define PHY_GEN_MAX_3_0         (0x1 << 10)
> +#define PHY_GEN_MAX_6_0         (0x2 << 10)
> +
> +#define BERLIN_SATA_PHY_NB	2
> +
> +#define to_berlin_sata_phy_priv(desc)	\
> +	container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
> +
> +struct phy_berlin_desc {
> +	struct phy	*phy;
> +	u32		val;
> +	unsigned	index;
> +};
> +
> +struct phy_berlin_priv {
> +	void __iomem		*base;
> +	spinlock_t		lock;
> +	struct phy_berlin_desc	phys[BERLIN_SATA_PHY_NB];
> +};
> +
> +static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
> +					       u32 mask, u32 val)
> +{
> +	u32 regval;
> +
> +	/* select register */
> +	writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
> +
> +	/* set bits */
> +	regval = readl(ctrl_reg + PORT_VSR_DATA);
> +	regval &= ~mask;
> +	regval |= val;
> +	writel(regval, ctrl_reg + PORT_VSR_DATA);
> +}
> +
> +static int phy_berlin_sata_power_on(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> +	void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
> +	int ret = 0;
> +	u32 regval;
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power on PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval &= ~(desc->val);
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* Configure MBus */
> +	writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	/* set PHY mode and ref freq to 25 MHz */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
> +				    REF_FREF_SEL_25 | PHY_MODE_SATA);
> +
> +	/* set PHY up to 6 Gbps */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
> +
> +	/* set 40 bits width */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
> +
> +	/* use max pll rate */
> +	phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
> +
> +	/* set the controller speed */
> +	writel(0x31, ctrl_reg + PORT_SCR_CTL);
> +
> +	spin_unlock(&priv->lock);
> +
> +	return ret;
> +}
> +
> +static int phy_berlin_sata_power_off(struct phy *phy)
> +{
> +	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
> +	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
> +	u32 regval;
> +
> +	spin_lock(&priv->lock);
> +
> +	/* Power down PHY */
> +	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
> +	regval = readl(priv->base + HOST_VSA_DATA);
> +	regval |= desc->val;
> +	writel(regval, priv->base + HOST_VSA_DATA);
> +
> +	spin_unlock(&priv->lock);
> +
> +	return 0;
> +}
> +
> +static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
> +					     struct of_phandle_args *args)
> +{
> +	struct phy_berlin_priv *priv = dev_get_drvdata(dev);
> +
> +	if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
> +		return ERR_PTR(-ENODEV);
> +
> +	return priv->phys[args->args[0]].phy;
> +}
> +
> +static struct phy_ops phy_berlin_sata_ops = {
> +	.power_on	= phy_berlin_sata_power_on,
> +	.power_off	= phy_berlin_sata_power_off,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static struct phy_berlin_desc desc[] = {
> +	{ .val = POWER_DOWN_PHY0 },
> +	{ .val = POWER_DOWN_PHY1 },

Only .val entry of struct phy_berlin_desc is initialized and needed,
it seems that u32 vals[] should be used instead of desc[].

> +	{ },
> +};
> +
> +static int phy_berlin_sata_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct phy *phy;
> +	struct phy_provider *phy_provider;
> +	struct phy_berlin_priv *priv;
> +	struct resource *res;
> +	int i;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);

devm_ioremap() (contrary to devm_ioremap_resource()) returns a valid
pointer or NULL so return value checking should be fixed.

> +	dev_set_drvdata(dev, priv);
> +	spin_lock_init(&priv->lock);
> +
> +	for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
> +		phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
> +		if (IS_ERR(phy)) {
> +			dev_err(dev, "failed to create PHY %d\n", i);
> +			return PTR_ERR(phy);
> +		}
> +
> +		priv->phys[i].phy = phy;
> +		priv->phys[i].val = desc[i].val;
> +		priv->phys[i].index = i;
> +		phy_set_drvdata(phy, &priv->phys[i]);
> +
> +		/* Make sure the PHY is off */
> +		phy_berlin_sata_power_off(phy);
> +	}
> +
> +	phy_provider =
> +		devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id phy_berlin_sata_of_match[] = {
> +	{ .compatible = "marvell,berlin-sata-phy" },
> +	{ },
> +};
> +
> +static struct platform_driver phy_berlin_sata_driver = {
> +	.probe	= phy_berlin_sata_probe,
> +	.driver	= {
> +		.name		= "phy-berlin-sata",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= phy_berlin_sata_of_match,
> +	},
> +};
> +module_platform_driver(phy_berlin_sata_driver);
> +
> +MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
> +MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
> +MODULE_LICENSE("GPL v2");

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Andrew Lunn May 20, 2014, 1:49 p.m. UTC | #4 
On Tue, May 20, 2014 at 02:34:20PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi,
> 
> Few minor issues below..
> 
> On Tuesday, May 20, 2014 11:04:29 AM Antoine Ténart wrote:
> > The Berlin SoC has a two SATA ports. Add a PHY driver to handle them.
> > 
> > The mode selection can let us think this PHY can be configured to fit
> > other purposes. But there are reasons to think the SATA mode will be
> > the only one usable: the PHY registers are only accessible indirectly
> > through two registers in the SATA range, the PHY seems to be integrated
> > and no information tells us the contrary. For these reasons, make the
> > driver a SATA PHY driver.
> > 
> > Signed-off-by: Antoine Ténart <antoine.tenart@free-electrons.com>
> > ---
> >  drivers/phy/Kconfig           |   5 +
> >  drivers/phy/Makefile          |   1 +
> >  drivers/phy/phy-berlin-sata.c | 230 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 236 insertions(+)
> >  create mode 100644 drivers/phy/phy-berlin-sata.c
> > 
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 4906c27fa3bd..b31b1986fda4 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -15,6 +15,11 @@ config GENERIC_PHY
> >  	  phy users can obtain reference to the PHY. All the users of this
> >  	  framework should select this config.
> >  
> > +config PHY_BERLIN_SATA
> > +	bool
> 
> Is there any real reason why this cannot be tristate?

What we have seen with SATA drivers and phys, is there is link time
breakage if the SATA driver is built in and the phy is modular.

Maybe this has been fixed now? 
 
	Andrew
Antoine Tenart May 20, 2014, 2:03 p.m. UTC | #5 
On Tue, May 20, 2014 at 03:49:42PM +0200, Andrew Lunn wrote:
> On Tue, May 20, 2014 at 02:34:20PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday, May 20, 2014 11:04:29 AM Antoine Ténart wrote:
> > > 
> > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > > index 4906c27fa3bd..b31b1986fda4 100644
> > > --- a/drivers/phy/Kconfig
> > > +++ b/drivers/phy/Kconfig
> > > @@ -15,6 +15,11 @@ config GENERIC_PHY
> > >  	  phy users can obtain reference to the PHY. All the users of this
> > >  	  framework should select this config.
> > >  
> > > +config PHY_BERLIN_SATA
> > > +	bool
> > 
> > Is there any real reason why this cannot be tristate?
> 
> What we have seen with SATA drivers and phys, is there is link time
> breakage if the SATA driver is built in and the phy is modular.
> 
> Maybe this has been fixed now? 

Using tristate shouldn't be a problem. I compiled without the PHY
driver, no link issue.

Antoine
Andrew Lunn May 20, 2014, 2:06 p.m. UTC | #6 
> > > > +config PHY_BERLIN_SATA
> > > > +	bool
> > > 
> > > Is there any real reason why this cannot be tristate?
> > 
> > What we have seen with SATA drivers and phys, is there is link time
> > breakage if the SATA driver is built in and the phy is modular.
> > 
> > Maybe this has been fixed now? 
> 
> Using tristate shouldn't be a problem. I compiled without the PHY
> driver, no link issue.

The problem i think was when the PHY core and driver was a module and
SATA built in. Please give that configuration a go.

     Andrew
Antoine Tenart May 20, 2014, 2:06 p.m. UTC | #7 
Hi,

On Tue, May 20, 2014 at 02:34:20PM +0200, Bartlomiej Zolnierkiewicz wrote:
> On Tuesday, May 20, 2014 11:04:29 AM Antoine Ténart wrote:
> > +
> > +static struct phy_berlin_desc desc[] = {
> > +	{ .val = POWER_DOWN_PHY0 },
> > +	{ .val = POWER_DOWN_PHY1 },
> 
> Only .val entry of struct phy_berlin_desc is initialized and needed,
> it seems that u32 vals[] should be used instead of desc[].

Sure.

> > +
> > +static int phy_berlin_sata_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct phy *phy;
> > +	struct phy_provider *phy_provider;
> > +	struct phy_berlin_priv *priv;
> > +	struct resource *res;
> > +	int i;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	priv->base = devm_ioremap(dev, res->start, resource_size(res));
> > +	if (IS_ERR(priv->base))
> > +		return PTR_ERR(priv->base);
> 
> devm_ioremap() (contrary to devm_ioremap_resource()) returns a valid
> pointer or NULL so return value checking should be fixed.

I'll fix this.


Thanks for the review!

Antoine
Antoine Tenart May 20, 2014, 2:40 p.m. UTC | #8 
On Tue, May 20, 2014 at 04:06:31PM +0200, Andrew Lunn wrote:
> > > > > +config PHY_BERLIN_SATA
> > > > > +	bool
> > > > 
> > > > Is there any real reason why this cannot be tristate?
> > > 
> > > What we have seen with SATA drivers and phys, is there is link time
> > > breakage if the SATA driver is built in and the phy is modular.
> > > 
> > > Maybe this has been fixed now? 
> > 
> > Using tristate shouldn't be a problem. I compiled without the PHY
> > driver, no link issue.
> 
> The problem i think was when the PHY core and driver was a module and
> SATA built in. Please give that configuration a go.

I tested, it went fine.

This was actually fixed by:
b51fbf9fb0c3 phy-core: Don't allow building phy-core as a module

Antoine
Antoine Tenart May 20, 2014, 2:40 p.m. UTC | #9 
On Tue, May 20, 2014 at 04:06:52PM +0200, Antoine Ténart wrote:
> Hi,
> 
> On Tue, May 20, 2014 at 02:34:20PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday, May 20, 2014 11:04:29 AM Antoine Ténart wrote:
> > > +
> > > +static struct phy_berlin_desc desc[] = {
> > > +	{ .val = POWER_DOWN_PHY0 },
> > > +	{ .val = POWER_DOWN_PHY1 },
> > 
> > Only .val entry of struct phy_berlin_desc is initialized and needed,
> > it seems that u32 vals[] should be used instead of desc[].
> 
> Sure.
> 
> > > +
> > > +static int phy_berlin_sata_probe(struct platform_device *pdev)
> > > +{
> > > +	struct device *dev = &pdev->dev;
> > > +	struct phy *phy;
> > > +	struct phy_provider *phy_provider;
> > > +	struct phy_berlin_priv *priv;
> > > +	struct resource *res;
> > > +	int i;
> > > +
> > > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > > +	if (!priv)
> > > +		return -ENOMEM;
> > > +
> > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

We also need to check res isn't NULL here.

> > > +	priv->base = devm_ioremap(dev, res->start, resource_size(res));
> > > +	if (IS_ERR(priv->base))
> > > +		return PTR_ERR(priv->base);
> > 
> > devm_ioremap() (contrary to devm_ioremap_resource()) returns a valid
> > pointer or NULL so return value checking should be fixed.
> 
> I'll fix this.

Antoine
diff mbox

Patch

diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 4906c27fa3bd..b31b1986fda4 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,6 +15,11 @@  config GENERIC_PHY
 	  phy users can obtain reference to the PHY. All the users of this
 	  framework should select this config.
 
+config PHY_BERLIN_SATA
+	bool
+	depends on ARCH_BERLIN && OF
+	select GENERIC_PHY
+
 config PHY_EXYNOS_MIPI_VIDEO
 	tristate "S5P/EXYNOS SoC series MIPI CSI-2/DSI PHY driver"
 	depends on HAS_IOMEM
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 7728518572a4..40278706ac1b 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@ 
 #
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
+obj-$(CONFIG_PHY_BERLIN_SATA)		+= phy-berlin-sata.o
 obj-$(CONFIG_BCM_KONA_USB2_PHY)		+= phy-bcm-kona-usb2.o
 obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
diff --git a/drivers/phy/phy-berlin-sata.c b/drivers/phy/phy-berlin-sata.c
new file mode 100644
index 000000000000..597f008cae32
--- /dev/null
+++ b/drivers/phy/phy-berlin-sata.c
@@ -0,0 +1,230 @@ 
+/*
+ * Marvell Berlin SATA PHY driver
+ *
+ * Copyright (C) 2014 Marvell Technology Group Ltd.
+ *
+ * Antoine Ténart <antoine.tenart@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/phy/phy.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+
+#define HOST_VSA_ADDR		0x0
+#define HOST_VSA_DATA		0x4
+#define PORT_VSR_ADDR           0x78
+#define PORT_VSR_DATA           0x7c
+#define PORT_SCR_CTL		0x2c
+
+#define CONTROL_REGISTER	0x0
+#define MBUS_SIZE_CONTROL	0x4
+
+#define POWER_DOWN_PHY0			BIT(6)
+#define POWER_DOWN_PHY1			BIT(14)
+#define MBUS_WRITE_REQUEST_SIZE_128	(BIT(2) << 16)
+#define MBUS_READ_REQUEST_SIZE_128	(BIT(2) << 19)
+
+#define PHY_BASE                0x200
+
+/* register 0x01 */
+#define REF_FREF_SEL_25         BIT(0)
+#define PHY_MODE_SATA           (0x0 << 5)
+
+/* register 0x02 */
+#define USE_MAX_PLL_RATE        BIT(12)
+
+/* register 0x23 */
+#define DATA_BIT_WIDTH_10       (0x0 << 10)
+#define DATA_BIT_WIDTH_20       (0x1 << 10)
+#define DATA_BIT_WIDTH_40       (0x2 << 10)
+
+/* register 0x25 */
+#define PHY_GEN_MAX_1_5         (0x0 << 10)
+#define PHY_GEN_MAX_3_0         (0x1 << 10)
+#define PHY_GEN_MAX_6_0         (0x2 << 10)
+
+#define BERLIN_SATA_PHY_NB	2
+
+#define to_berlin_sata_phy_priv(desc)	\
+	container_of((desc), struct phy_berlin_priv, phys[(desc)->index])
+
+struct phy_berlin_desc {
+	struct phy	*phy;
+	u32		val;
+	unsigned	index;
+};
+
+struct phy_berlin_priv {
+	void __iomem		*base;
+	spinlock_t		lock;
+	struct phy_berlin_desc	phys[BERLIN_SATA_PHY_NB];
+};
+
+static inline void phy_berlin_sata_reg_setbits(void __iomem *ctrl_reg, u32 reg,
+					       u32 mask, u32 val)
+{
+	u32 regval;
+
+	/* select register */
+	writel(PHY_BASE + reg, ctrl_reg + PORT_VSR_ADDR);
+
+	/* set bits */
+	regval = readl(ctrl_reg + PORT_VSR_DATA);
+	regval &= ~mask;
+	regval |= val;
+	writel(regval, ctrl_reg + PORT_VSR_DATA);
+}
+
+static int phy_berlin_sata_power_on(struct phy *phy)
+{
+	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
+	void __iomem *ctrl_reg = priv->base + 0x60 + (desc->index * 0x80);
+	int ret = 0;
+	u32 regval;
+
+	spin_lock(&priv->lock);
+
+	/* Power on PHY */
+	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval &= ~(desc->val);
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	/* Configure MBus */
+	writel(MBUS_SIZE_CONTROL, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval |= MBUS_WRITE_REQUEST_SIZE_128 | MBUS_READ_REQUEST_SIZE_128;
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	/* set PHY mode and ref freq to 25 MHz */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x1, 0xff,
+				    REF_FREF_SEL_25 | PHY_MODE_SATA);
+
+	/* set PHY up to 6 Gbps */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x25, 0xc00, PHY_GEN_MAX_6_0);
+
+	/* set 40 bits width */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x23,  0xc00, DATA_BIT_WIDTH_40);
+
+	/* use max pll rate */
+	phy_berlin_sata_reg_setbits(ctrl_reg, 0x2, 0x0, USE_MAX_PLL_RATE);
+
+	/* set the controller speed */
+	writel(0x31, ctrl_reg + PORT_SCR_CTL);
+
+	spin_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int phy_berlin_sata_power_off(struct phy *phy)
+{
+	struct phy_berlin_desc *desc = phy_get_drvdata(phy);
+	struct phy_berlin_priv *priv = to_berlin_sata_phy_priv(desc);
+	u32 regval;
+
+	spin_lock(&priv->lock);
+
+	/* Power down PHY */
+	writel(CONTROL_REGISTER, priv->base + HOST_VSA_ADDR);
+	regval = readl(priv->base + HOST_VSA_DATA);
+	regval |= desc->val;
+	writel(regval, priv->base + HOST_VSA_DATA);
+
+	spin_unlock(&priv->lock);
+
+	return 0;
+}
+
+static struct phy *phy_berlin_sata_phy_xlate(struct device *dev,
+					     struct of_phandle_args *args)
+{
+	struct phy_berlin_priv *priv = dev_get_drvdata(dev);
+
+	if (WARN_ON(args->args[0] >= BERLIN_SATA_PHY_NB))
+		return ERR_PTR(-ENODEV);
+
+	return priv->phys[args->args[0]].phy;
+}
+
+static struct phy_ops phy_berlin_sata_ops = {
+	.power_on	= phy_berlin_sata_power_on,
+	.power_off	= phy_berlin_sata_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy_berlin_desc desc[] = {
+	{ .val = POWER_DOWN_PHY0 },
+	{ .val = POWER_DOWN_PHY1 },
+	{ },
+};
+
+static int phy_berlin_sata_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	struct phy_provider *phy_provider;
+	struct phy_berlin_priv *priv;
+	struct resource *res;
+	int i;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap(dev, res->start, resource_size(res));
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	dev_set_drvdata(dev, priv);
+	spin_lock_init(&priv->lock);
+
+	for (i = 0; i < BERLIN_SATA_PHY_NB; i++) {
+		phy = devm_phy_create(dev, &phy_berlin_sata_ops, NULL);
+		if (IS_ERR(phy)) {
+			dev_err(dev, "failed to create PHY %d\n", i);
+			return PTR_ERR(phy);
+		}
+
+		priv->phys[i].phy = phy;
+		priv->phys[i].val = desc[i].val;
+		priv->phys[i].index = i;
+		phy_set_drvdata(phy, &priv->phys[i]);
+
+		/* Make sure the PHY is off */
+		phy_berlin_sata_power_off(phy);
+	}
+
+	phy_provider =
+		devm_of_phy_provider_register(dev, phy_berlin_sata_phy_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	return 0;
+}
+
+static const struct of_device_id phy_berlin_sata_of_match[] = {
+	{ .compatible = "marvell,berlin-sata-phy" },
+	{ },
+};
+
+static struct platform_driver phy_berlin_sata_driver = {
+	.probe	= phy_berlin_sata_probe,
+	.driver	= {
+		.name		= "phy-berlin-sata",
+		.owner		= THIS_MODULE,
+		.of_match_table	= phy_berlin_sata_of_match,
+	},
+};
+module_platform_driver(phy_berlin_sata_driver);
+
+MODULE_DESCRIPTION("Marvell Berlin SATA PHY driver");
+MODULE_AUTHOR("Antoine Ténart <antoine.tenart@free-electrons.com>");
+MODULE_LICENSE("GPL v2");