Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
On Tue, Apr 07, 2015 at 11:37:43AM +0530, Kishon Vijay Abraham I wrote:
> On Thursday 02 April 2015 07:58 AM, Brian Norris wrote:
> >On Tue, Mar 31, 2015 at 11:31:40AM +0530, Kishon Vijay Abraham I wrote:
> >>On Saturday 28 March 2015 05:58 AM, Brian Norris wrote:
> >>>On Thu, Mar 26, 2015 at 03:29:44AM +0530, Kishon Vijay Abraham I wrote:
> On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
> >+static struct phy *brcm_sata_phy_xlate(struct device *dev,
> >+ struct of_phandle_args *args)
> >+{
> >+struct brcm_sata_phy *priv = dev_get_drvdata(dev);
> >+int i = args->args[0];
> >+
> >+if (i >= MAX_PORTS || !priv->phys[i].phy) {
> >+dev_err(dev, "invalid phy: %d\n", i);
> >+return ERR_PTR(-ENODEV);
> >+}
> >+
> >+return priv->phys[i].phy;
> >+}
>
> this xlate is not required at all if the controller device tree node has
> phandle to the phy node (sub node) instead of the phy provider device tree
> node.
> >>>
> >>>That doesn't match any convention I see in existing SATA phy bindings,
> >>>nor do I see how the existing of_phy_simple_xlate() would support this,
> >>>unless I instantiate a device for each port's PHY. If I adjust the
> >>>device tree as you suggest, and use of_phy_simple_xlate() instead of
> >>>this, of_phy_get() can't find the PHY provider, because the provider is
> >>>registered to the parent, not the subnode.
> >>
> >>The phy core should still be able to get the PHY provider.
> >>See this in of_phy_provider_lookup
> >> for_each_child_of_node(phy_provider->dev->of_node, child)
> >> if (child == node)
> >> return phy_provider;
> >
> >That just searches for children of the node. It doesn't walk parent
> >nodes.
>
> okay.. in your phy_create pass the np of the PHYs (sub-node pointer
> to phy provider).
Ah, I see. I completely passed over the 2nd parameter to phy_create()...
Thanks for the tip.
> >>Can you post your device tree node here?
> >
> >You mean patch 5?
> >
> >https://lkml.org/lkml/2015/3/18/937
> >
> >>>
> >>>Can you elaborate on your suggestion?
>
> Change the dt node to something like below..
[snip]
Yes, that worked. Thanks.
OK, I'll fix this up and send out v2 shortly.
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
On Thursday 02 April 2015 07:58 AM, Brian Norris wrote:
On Tue, Mar 31, 2015 at 11:31:40AM +0530, Kishon Vijay Abraham I wrote:
On Saturday 28 March 2015 05:58 AM, Brian Norris wrote:
On Thu, Mar 26, 2015 at 03:29:44AM +0530, Kishon Vijay Abraham I wrote:
On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
+static struct phy *brcm_sata_phy_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct brcm_sata_phy *priv = dev_get_drvdata(dev);
+ int i = args->args[0];
+
+ if (i >= MAX_PORTS || !priv->phys[i].phy) {
+ dev_err(dev, "invalid phy: %d\n", i);
+ return ERR_PTR(-ENODEV);
+ }
+
+ return priv->phys[i].phy;
+}
this xlate is not required at all if the controller device tree node has
phandle to the phy node (sub node) instead of the phy provider device tree
node.
That doesn't match any convention I see in existing SATA phy bindings,
nor do I see how the existing of_phy_simple_xlate() would support this,
unless I instantiate a device for each port's PHY. If I adjust the
device tree as you suggest, and use of_phy_simple_xlate() instead of
this, of_phy_get() can't find the PHY provider, because the provider is
registered to the parent, not the subnode.
The phy core should still be able to get the PHY provider.
See this in of_phy_provider_lookup
for_each_child_of_node(phy_provider->dev->of_node, child)
if (child == node)
return phy_provider;
That just searches for children of the node. It doesn't walk parent
nodes.
okay.. in your phy_create pass the np of the PHYs (sub-node pointer to phy
provider).
Can you post your device tree node here?
You mean patch 5?
https://lkml.org/lkml/2015/3/18/937
Can you elaborate on your suggestion?
Change the dt node to something like below..
+
+ sata@f045a000 {
+
+
+ sata0: sata-port@0 {
+ reg = <0>;
+ phys = <&sata_phy0>;
+ };
+
+ sata1: sata-port@1 {
+ reg = <1>;
+ phys = <&sata_phy1>;
+ };
+ };
+
+ sata_phy: sata-phy@f0458100 {
+ compatible = "brcm,bcm7445-sata-phy", "brcm,phy-sata3";
+ reg = <0x458100 0x1e00>, <0x45804c 0x10>;
+ reg-names = "phy", "port-ctrl";
+ #address-cells = <0x1>;
+ #size-cells = <0x0>;
+
+ sata_phy0: sata-phy@0 {
+ reg = <0>;
+ #phy-cells = <0>;
+ };
+
+ sata_phy1: sata-phy@1 {
+ #phy-cells = <0>;
+ };
+ };
};
+
+static const struct of_device_id brcmstb_sata_phy_of_match[] = {
+ { .compatible = "brcm,bcm7445-sata-phy" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, brcmstb_sata_phy_of_match);
+
+static int brcmstb_sata_phy_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *dn = dev->of_node, *child;
+ struct brcm_sata_phy *priv;
+ struct resource *res;
+ struct phy_provider *provider;
+ int count = 0;
+
+ if (of_get_child_count(dn) == 0)
+ return -ENODEV;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ dev_set_drvdata(dev, priv);
+ priv->dev = dev;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "port-ctrl");
+ if (!res) {
+ dev_err(dev, "couldn't get port-ctrl resource\n");
+ return -EINVAL;
+ }
+ /*
+* Don't request region, since it may be within a region owned by the
+* SATA driver
It should be in the SATA driver then. Why is it here?
Did you read the discussion branching here?
http://article.gmane.org/gmane.linux.drivers.devicetree/114637
I've seen the exact opposite suggestion already (move it to the PHY
driver), and I'm not sure either suggestion is correct. The same
register block has registers for both the PHY and the SATA controller.
IMHO the register space shouldn't be defined based on the programming sequence
but by where the register is actually present in the IP. From that thread it
doesn't look like it is present in the PHY IP.
If you say so. But it has plenty of PHY controls packed into those
registers, so are you just recommending handling those bits from the
SATA driver?
Yes. Let's program only the PHY IP in this driver.
...
lets not make the boot noisy. Make it dev_vdbg if it is required.
I think it's important to have at least some of the three informational
print
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
On Tue, Mar 31, 2015 at 11:31:40AM +0530, Kishon Vijay Abraham I wrote:
> On Saturday 28 March 2015 05:58 AM, Brian Norris wrote:
> >On Thu, Mar 26, 2015 at 03:29:44AM +0530, Kishon Vijay Abraham I wrote:
> >>On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
> >>>+static struct phy *brcm_sata_phy_xlate(struct device *dev,
> >>>+ struct of_phandle_args *args)
> >>>+{
> >>>+ struct brcm_sata_phy *priv = dev_get_drvdata(dev);
> >>>+ int i = args->args[0];
> >>>+
> >>>+ if (i >= MAX_PORTS || !priv->phys[i].phy) {
> >>>+ dev_err(dev, "invalid phy: %d\n", i);
> >>>+ return ERR_PTR(-ENODEV);
> >>>+ }
> >>>+
> >>>+ return priv->phys[i].phy;
> >>>+}
> >>
> >>this xlate is not required at all if the controller device tree node has
> >>phandle to the phy node (sub node) instead of the phy provider device tree
> >>node.
> >
> >That doesn't match any convention I see in existing SATA phy bindings,
> >nor do I see how the existing of_phy_simple_xlate() would support this,
> >unless I instantiate a device for each port's PHY. If I adjust the
> >device tree as you suggest, and use of_phy_simple_xlate() instead of
> >this, of_phy_get() can't find the PHY provider, because the provider is
> >registered to the parent, not the subnode.
>
> The phy core should still be able to get the PHY provider.
> See this in of_phy_provider_lookup
> for_each_child_of_node(phy_provider->dev->of_node, child)
> if (child == node)
> return phy_provider;
That just searches for children of the node. It doesn't walk parent
nodes.
> Can you post your device tree node here?
You mean patch 5?
https://lkml.org/lkml/2015/3/18/937
> >
> >Can you elaborate on your suggestion?
> >
> >>>+
> >>>+static const struct of_device_id brcmstb_sata_phy_of_match[] = {
> >>>+ { .compatible = "brcm,bcm7445-sata-phy" },
> >>>+ {},
> >>>+};
> >>>+MODULE_DEVICE_TABLE(of, brcmstb_sata_phy_of_match);
> >>>+
> >>>+static int brcmstb_sata_phy_probe(struct platform_device *pdev)
> >>>+{
> >>>+ struct device *dev = &pdev->dev;
> >>>+ struct device_node *dn = dev->of_node, *child;
> >>>+ struct brcm_sata_phy *priv;
> >>>+ struct resource *res;
> >>>+ struct phy_provider *provider;
> >>>+ int count = 0;
> >>>+
> >>>+ if (of_get_child_count(dn) == 0)
> >>>+ return -ENODEV;
> >>>+
> >>>+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >>>+ if (!priv)
> >>>+ return -ENOMEM;
> >>>+ dev_set_drvdata(dev, priv);
> >>>+ priv->dev = dev;
> >>>+
> >>>+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "port-ctrl");
> >>>+ if (!res) {
> >>>+ dev_err(dev, "couldn't get port-ctrl resource\n");
> >>>+ return -EINVAL;
> >>>+ }
> >>>+ /*
> >>>+ * Don't request region, since it may be within a region owned by the
> >>>+ * SATA driver
> >>
> >>It should be in the SATA driver then. Why is it here?
> >
> >Did you read the discussion branching here?
> >
> >http://article.gmane.org/gmane.linux.drivers.devicetree/114637
> >
> >I've seen the exact opposite suggestion already (move it to the PHY
> >driver), and I'm not sure either suggestion is correct. The same
> >register block has registers for both the PHY and the SATA controller.
>
> IMHO the register space shouldn't be defined based on the programming sequence
> but by where the register is actually present in the IP. From that thread it
> doesn't look like it is present in the PHY IP.
If you say so. But it has plenty of PHY controls packed into those
registers, so are you just recommending handling those bits from the
SATA driver?
...
> >>lets not make the boot noisy. Make it dev_vdbg if it is required.
> >
> >I think it's important to have at least some of the three informational
> >prints that you're suggesting turn into dbg. It's pretty important to
> >see that we're powering on one or more PHY ports, for both
> >power/correctness concerns (trying to power on a port that is
> >OTP-disabled, for instance) and debugging concerns (the suggestions you
> >made about the device tree yielded a dead SATA, and it was obvious,
> >because the "powering on" prints were missing).
>
> All these are debugging info. Hence it's better to keep in dev_vdbg or
> dev_dbg.
> >
> >I'd kinda like to see the previous power on/off prints above stay as
> >dev_info(), though the "registered" print might be superfluous, as the
> >registration info should show up in sysfs.
> >
> >Related: I don't see any API for monitoring the PHY status from user
> >space. e.g., there's nothing useful in /sys/class/phy/*/.
>
> Do you really need to monitor the PHY status? We should be careful about
> exposing anything to the user space since it will become an ABI and we can
> never modify it.
Not really, but the debugging info (which you want me to remove by
default, and which is unretrievable after system boot) is the next
easiest solution. It doesn't provide an ABI, exa
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
Hi,
On Saturday 28 March 2015 05:58 AM, Brian Norris wrote:
Hi Kishon,
On Thu, Mar 26, 2015 at 03:29:44AM +0530, Kishon Vijay Abraham I wrote:
On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
Supports up to two ports which can each be powered on/off and configured
independently.
Signed-off-by: Brian Norris
---
drivers/phy/Kconfig| 9 ++
drivers/phy/Makefile | 1 +
drivers/phy/phy-brcmstb-sata.c | 333 +
3 files changed, 343 insertions(+)
create mode 100644 drivers/phy/phy-brcmstb-sata.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 2962de205ba7..c8b22074bcf6 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -291,4 +291,13 @@ config PHY_QCOM_UFS
help
Support for UFS PHY on QCOM chipsets.
+config PHY_BRCMSTB_SATA
+ tristate "Broadcom STB SATA PHY driver"
+ depends on ARCH_BRCMSTB
+ depends on OF
+ select GENERIC_PHY
+ help
+ Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
+ Likely useful only with CONFIG_SATA_BRCMSTB enabled.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index f080e1bb2a74..28a10804b4f4 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o
obj-$(CONFIG_PHY_QCOM_UFS)+= phy-qcom-ufs.o
obj-$(CONFIG_PHY_QCOM_UFS)+= phy-qcom-ufs-qmp-20nm.o
obj-$(CONFIG_PHY_QCOM_UFS)+= phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_BRCMSTB_SATA) += phy-brcmstb-sata.o
diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
new file mode 100644
index ..413bc94225ac
--- /dev/null
+++ b/drivers/phy/phy-brcmstb-sata.c
@@ -0,0 +1,333 @@
+/*
+ * Broadcom SATA3 AHCI Controller PHY Driver
+ *
+ * Copyright © 2009-2015 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SATA_MDIO_BANK_OFFSET 0x23c
+#define SATA_MDIO_REG_OFFSET(ofs) ((ofs) * 4)
+#define SATA_MDIO_REG_SPACE_SIZE 0x1000
+#define SATA_MDIO_REG_LENGTH 0x1f00
+
+#define SATA_TOP_CTRL_PHY_CTRL_1 0x0
+ #define SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE BIT(14)
+
+#define SATA_TOP_CTRL_PHY_CTRL_2 0x4
+ #define SATA_TOP_CTRL_2_SW_RST_MDIOREGBIT(0)
+ #define SATA_TOP_CTRL_2_SW_RST_OOBBIT(1)
+ #define SATA_TOP_CTRL_2_SW_RST_RX BIT(2)
+ #define SATA_TOP_CTRL_2_SW_RST_TX BIT(3)
+ #define SATA_TOP_CTRL_2_PHY_GLOBAL_RESET BIT(14)
+
+#define MAX_PORTS 2
+/* Register offset between PHYs in port-ctrl */
+#define SATA_TOP_CTRL_PHY_CTRL_LEN 0x8
+/* Register offset between PHYs in PCB space */
+#define SATA_MDIO_REG_SPACE_SIZE 0x1000
+
+struct brcm_sata_port {
+ int portnum;
+ struct phy *phy;
+ struct brcm_sata_phy *phy_priv;
+ bool ssc_en;
+};
+
+struct brcm_sata_phy {
+ struct device *dev;
+ void __iomem *port_ctrl;
+ void __iomem *phy_base;
+
+ struct brcm_sata_port phys[MAX_PORTS];
Can't we allocate memory for the PHYs dynamically?
Yes, but I don't see a lot of point for an IP that supports exactly 2
ports...
+};
+
+enum sata_mdio_phy_regs_28nm {
+ PLL_REG_BANK_0 = 0x50,
+ PLL_REG_BANK_0_PLLCONTROL_0 = 0x81,
+
+ TXPMD_REG_BANK = 0x1a0,
+ TXPMD_CONTROL1 = 0x81,
+ TXPMD_CONTROL1_TX_SSC_EN_FRC= BIT(0),
+ TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL= BIT(1),
+ TXPMD_TX_FREQ_CTRL_CONTROL1 = 0x82,
+ TXPMD_TX_FREQ_CTRL_CONTROL2 = 0x83,
+ TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK = 0x3ff,
+ TXPMD_TX_FREQ_CTRL_CONTROL3 = 0x84,
+ TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK = 0x3ff,
+};
+
+static inline void __iomem *sata_phy_get_port_ctrl(struct brcm_sata_port *port)
+{
+ struct brcm_sata_phy *priv = port->phy_priv;
+
+ return priv->port_ctrl + (port->portnum * SATA_TOP_CTRL_PHY_CTRL_LEN);
+}
+static inline void __iomem *sata_phy_get_phy_base(struct brcm_sata_port *port)
please use consistent n
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
Hi Kishon,
On Thu, Mar 26, 2015 at 03:29:44AM +0530, Kishon Vijay Abraham I wrote:
> On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
> > Supports up to two ports which can each be powered on/off and configured
> > independently.
> >
> > Signed-off-by: Brian Norris
> > ---
> > drivers/phy/Kconfig| 9 ++
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-brcmstb-sata.c | 333
> > +
> > 3 files changed, 343 insertions(+)
> > create mode 100644 drivers/phy/phy-brcmstb-sata.c
> >
> > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> > index 2962de205ba7..c8b22074bcf6 100644
> > --- a/drivers/phy/Kconfig
> > +++ b/drivers/phy/Kconfig
> > @@ -291,4 +291,13 @@ config PHY_QCOM_UFS
> > help
> > Support for UFS PHY on QCOM chipsets.
> >
> > +config PHY_BRCMSTB_SATA
> > + tristate "Broadcom STB SATA PHY driver"
> > + depends on ARCH_BRCMSTB
> > + depends on OF
> > + select GENERIC_PHY
> > + help
> > + Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
> > + Likely useful only with CONFIG_SATA_BRCMSTB enabled.
> > +
> > endmenu
> > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> > index f080e1bb2a74..28a10804b4f4 100644
> > --- a/drivers/phy/Makefile
> > +++ b/drivers/phy/Makefile
> > @@ -38,3 +38,4 @@ obj-$(CONFIG_PHY_STIH41X_USB) +=
> > phy-stih41x-usb.o
> > obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
> > obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
> > obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
> > +obj-$(CONFIG_PHY_BRCMSTB_SATA) += phy-brcmstb-sata.o
> > diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
> > new file mode 100644
> > index ..413bc94225ac
> > --- /dev/null
> > +++ b/drivers/phy/phy-brcmstb-sata.c
> > @@ -0,0 +1,333 @@
> > +/*
> > + * Broadcom SATA3 AHCI Controller PHY Driver
> > + *
> > + * Copyright © 2009-2015 Broadcom Corporation
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2, or (at your option)
> > + * any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define SATA_MDIO_BANK_OFFSET 0x23c
> > +#define SATA_MDIO_REG_OFFSET(ofs) ((ofs) * 4)
> > +#define SATA_MDIO_REG_SPACE_SIZE 0x1000
> > +#define SATA_MDIO_REG_LENGTH 0x1f00
> > +
> > +#define SATA_TOP_CTRL_PHY_CTRL_1 0x0
> > + #define SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE BIT(14)
> > +
> > +#define SATA_TOP_CTRL_PHY_CTRL_2 0x4
> > + #define SATA_TOP_CTRL_2_SW_RST_MDIOREGBIT(0)
> > + #define SATA_TOP_CTRL_2_SW_RST_OOBBIT(1)
> > + #define SATA_TOP_CTRL_2_SW_RST_RX BIT(2)
> > + #define SATA_TOP_CTRL_2_SW_RST_TX BIT(3)
> > + #define SATA_TOP_CTRL_2_PHY_GLOBAL_RESET BIT(14)
> > +
> > +#define MAX_PORTS 2
> > +/* Register offset between PHYs in port-ctrl */
> > +#define SATA_TOP_CTRL_PHY_CTRL_LEN 0x8
> > +/* Register offset between PHYs in PCB space */
> > +#define SATA_MDIO_REG_SPACE_SIZE 0x1000
> > +
> > +struct brcm_sata_port {
> > + int portnum;
> > + struct phy *phy;
> > + struct brcm_sata_phy *phy_priv;
> > + bool ssc_en;
> > +};
> > +
> > +struct brcm_sata_phy {
> > + struct device *dev;
> > + void __iomem *port_ctrl;
> > + void __iomem *phy_base;
> > +
> > + struct brcm_sata_port phys[MAX_PORTS];
>
> Can't we allocate memory for the PHYs dynamically?
Yes, but I don't see a lot of point for an IP that supports exactly 2
ports...
> > +};
> > +
> > +enum sata_mdio_phy_regs_28nm {
> > + PLL_REG_BANK_0 = 0x50,
> > + PLL_REG_BANK_0_PLLCONTROL_0 = 0x81,
> > +
> > + TXPMD_REG_BANK = 0x1a0,
> > + TXPMD_CONTROL1 = 0x81,
> > + TXPMD_CONTROL1_TX_SSC_EN_FRC= BIT(0),
> > + TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL= BIT(1),
> > + TXPMD_TX_FREQ_CTRL_CONTROL1 = 0x82,
> > + TXPMD_TX_FREQ_CTRL_CONTROL2 = 0x83,
> > + TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK = 0x3ff,
> > + TXPMD_TX_FREQ_CTRL_CONTROL3 = 0x84,
> > + TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK = 0x3ff,
> > +};
> > +
> > +static inline void __i
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
Hi,
On Thursday 19 March 2015 06:53 AM, Brian Norris wrote:
> Supports up to two ports which can each be powered on/off and configured
> independently.
>
> Signed-off-by: Brian Norris
> ---
> drivers/phy/Kconfig| 9 ++
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-brcmstb-sata.c | 333
> +
> 3 files changed, 343 insertions(+)
> create mode 100644 drivers/phy/phy-brcmstb-sata.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 2962de205ba7..c8b22074bcf6 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -291,4 +291,13 @@ config PHY_QCOM_UFS
> help
> Support for UFS PHY on QCOM chipsets.
>
> +config PHY_BRCMSTB_SATA
> + tristate "Broadcom STB SATA PHY driver"
> + depends on ARCH_BRCMSTB
> + depends on OF
> + select GENERIC_PHY
> + help
> + Enable this to support the SATA3 PHY on 28nm Broadcom STB SoCs.
> + Likely useful only with CONFIG_SATA_BRCMSTB enabled.
> +
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index f080e1bb2a74..28a10804b4f4 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -38,3 +38,4 @@ obj-$(CONFIG_PHY_STIH41X_USB) +=
> phy-stih41x-usb.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
> obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
> +obj-$(CONFIG_PHY_BRCMSTB_SATA) += phy-brcmstb-sata.o
> diff --git a/drivers/phy/phy-brcmstb-sata.c b/drivers/phy/phy-brcmstb-sata.c
> new file mode 100644
> index ..413bc94225ac
> --- /dev/null
> +++ b/drivers/phy/phy-brcmstb-sata.c
> @@ -0,0 +1,333 @@
> +/*
> + * Broadcom SATA3 AHCI Controller PHY Driver
> + *
> + * Copyright © 2009-2015 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define SATA_MDIO_BANK_OFFSET0x23c
> +#define SATA_MDIO_REG_OFFSET(ofs)((ofs) * 4)
> +#define SATA_MDIO_REG_SPACE_SIZE 0x1000
> +#define SATA_MDIO_REG_LENGTH 0x1f00
> +
> +#define SATA_TOP_CTRL_PHY_CTRL_1 0x0
> + #define SATA_TOP_CTRL_1_PHY_DEFAULT_POWER_STATE BIT(14)
> +
> +#define SATA_TOP_CTRL_PHY_CTRL_2 0x4
> + #define SATA_TOP_CTRL_2_SW_RST_MDIOREG BIT(0)
> + #define SATA_TOP_CTRL_2_SW_RST_OOB BIT(1)
> + #define SATA_TOP_CTRL_2_SW_RST_RX BIT(2)
> + #define SATA_TOP_CTRL_2_SW_RST_TX BIT(3)
> + #define SATA_TOP_CTRL_2_PHY_GLOBAL_RESETBIT(14)
> +
> +#define MAX_PORTS2
> +/* Register offset between PHYs in port-ctrl */
> +#define SATA_TOP_CTRL_PHY_CTRL_LEN 0x8
> +/* Register offset between PHYs in PCB space */
> +#define SATA_MDIO_REG_SPACE_SIZE 0x1000
> +
> +struct brcm_sata_port {
> + int portnum;
> + struct phy *phy;
> + struct brcm_sata_phy *phy_priv;
> + bool ssc_en;
> +};
> +
> +struct brcm_sata_phy {
> + struct device *dev;
> + void __iomem *port_ctrl;
> + void __iomem *phy_base;
> +
> + struct brcm_sata_port phys[MAX_PORTS];
Can't we allocate memory for the PHYs dynamically?
> +};
> +
> +enum sata_mdio_phy_regs_28nm {
> + PLL_REG_BANK_0 = 0x50,
> + PLL_REG_BANK_0_PLLCONTROL_0 = 0x81,
> +
> + TXPMD_REG_BANK = 0x1a0,
> + TXPMD_CONTROL1 = 0x81,
> + TXPMD_CONTROL1_TX_SSC_EN_FRC= BIT(0),
> + TXPMD_CONTROL1_TX_SSC_EN_FRC_VAL= BIT(1),
> + TXPMD_TX_FREQ_CTRL_CONTROL1 = 0x82,
> + TXPMD_TX_FREQ_CTRL_CONTROL2 = 0x83,
> + TXPMD_TX_FREQ_CTRL_CONTROL2_FMIN_MASK = 0x3ff,
> + TXPMD_TX_FREQ_CTRL_CONTROL3 = 0x84,
> + TXPMD_TX_FREQ_CTRL_CONTROL3_FMAX_MASK = 0x3ff,
> +};
> +
> +static inline void __iomem *sata_phy_get_port_ctrl(struct brcm_sata_port
> *port)
> +{
> + struct brcm_sata_phy *priv = port->phy_priv;
> +
> + return priv->port_ctrl + (port->portnum * SATA_TOP_CTRL_PHY_CTRL_LEN);
> +}
> +static inline void __iomem *sata_phy_get_phy_base(struct brcm_sata_port
> *port)
please use consistent names for all functions. Prefix all the fun
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
Hi,
On 21-03-15 00:02, Florian Fainelli wrote:
On 18/03/15 18:23, Brian Norris wrote:
Supports up to two ports which can each be powered on/off and configured
independently.
Signed-off-by: Brian Norris
---
[snip]
+
+static const struct of_device_id brcmstb_sata_phy_of_match[] = {
+ { .compatible = "brcm,bcm7445-sata-phy" },
The binding document specifies "brcm,phy-sata3" as a fallback compatible
string, so we want to match it here.
Erm no we do not, what if another sata3 brcm phy comes along which
is not compatible, both would use "brcm,phy-sata3" as fallback / for
informational purposes, but we do not want this driver to be binding
to that other phy, which it would do with your suggestion.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] phy: add Broadcom SATA3 PHY driver for Broadcom STB SoCs
On 18/03/15 18:23, Brian Norris wrote:
> Supports up to two ports which can each be powered on/off and configured
> independently.
>
> Signed-off-by: Brian Norris
> ---
[snip]
> +
> +static const struct of_device_id brcmstb_sata_phy_of_match[] = {
> + { .compatible = "brcm,bcm7445-sata-phy" },
The binding document specifies "brcm,phy-sata3" as a fallback compatible
string, so we want to match it here.
[snip]
> +
> + dev_info(dev, "registered %d ports\n", count);
"registered %d port(s)" ;)
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
