Hi Kishon,
This submission is the same as the last one, only each channel is
now represented by its own sub-node. The documentation has also
been updated accordingly.
Kind regards,
Lee
.../devicetree/bindings/phy/phy-miphy365x.txt | 76 +++
arch/arm/boot/dts/stih416-b2020-revE.dts | 5 +
arch/arm/boot/dts/stih416-b2020.dts | 7 +
arch/arm/boot/dts/stih416.dtsi | 21 +
drivers/phy/Kconfig | 10 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-miphy365x.c | 676 +++++++++++++++++++++
include/dt-bindings/phy/phy-miphy365x.h | 25 +
8 files changed, 821 insertions(+)
The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
devices. It has 2 ports which it can use for either; both SATA, both
PCIe or one of each in any configuration.
Cc: Kishon Vijay Abraham I <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Acked-by: Alexandre Torgue <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
.../devicetree/bindings/phy/phy-miphy365x.txt | 76 ++++++++++++++++++++++
1 file changed, 76 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt
diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
new file mode 100644
index 0000000..d75f300
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
@@ -0,0 +1,76 @@
+STMicroelectronics STi MIPHY365x PHY binding
+============================================
+
+This binding describes a miphy device that is used to control PHY hardware
+for SATA and PCIe.
+
+Required properties:
+- compatible : Should be "st,miphy365x-phy"
+- #phy-cells : Should be 2 (See second example)
+ First cell is the port number from:
+ - MIPHY_PORT_0
+ - MIPHY_PORT_1
+ Second cell is device type from:
+ - MIPHY_TYPE_SATA
+ - MIPHY_TYPE_PCI
+- reg : Address and length of register sets for each device in
+ "reg-names"
+- reg-names : The names of the register addresses corresponding to the
+ registers filled in "reg", from:
+ - sata0: For SATA port 0 registers
+ - sata1: For SATA port 1 registers
+ - pcie0: For PCIE port 0 registers
+ - pcie1: For PCIE port 1 registers
+- st,syscfg : Should be a phandle of the system configuration register group
+ which contain the SATA, PCIe mode setting bits
+
+Optional properties:
+- st,sata-gen : Generation of locally attached SATA IP. Expected values
+ are {1,2,3). If not supplied generation 1 hardware will
+ be expected
+- st,pcie-tx-pol-inv : Bool property to invert the polarity PCIe Tx (Txn/Txp)
+- st,sata-tx-pol-inv : Bool property to invert the polarity SATA Tx (Txn/Txp)
+
+Required nodes : A sub-node is required for each channel the controller
+ provides. Address range information including the usual
+ 'reg' and 'reg-names' properties are used inside these
+ nodes to describe the controller's topology. These nodes
+ are translated by the driver's .xlate() function.
+
+Example:
+
+ miphy365x_phy: miphy365x@fe382000 {
+ compatible = "st,miphy365x-phy";
+ st,syscfg = <&syscfg_rear>;
+ #phy-cells = <2>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ phy_port0: port@fe382000 {
+ reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
+ reg-names = "sata", "pcie";
+ };
+
+ phy_port1: port@fe38a000 {
+ reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
+ reg-names = "sata", "pcie";
+ };
+ };
+
+Specifying phy control of devices
+=================================
+
+Device nodes should specify the configuration required in their "phys"
+property, containing a phandle to the miphy device node, a port number
+and a device type.
+
+Example:
+
+#include <dt-bindings/phy/phy-miphy365x.h>
+
+ sata0: sata@fe380000 {
+ ...
+ phys = <&miphy365x_phy MIPHY_PORT_0 MIPHY_TYPE_SATA>;
+ ...
+ };
--
1.8.3.2
The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
devices. It has 2 ports which it can use for either; both SATA, both
PCIe or one of each in any configuration.
Acked-by: Mark Rutland <[email protected]>
Acked-by: Alexandre Torgue <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/stih416-b2020-revE.dts | 5 +++++
arch/arm/boot/dts/stih416-b2020.dts | 7 +++++++
arch/arm/boot/dts/stih416.dtsi | 21 +++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
index ba0fa2c..0e2c870 100644
--- a/arch/arm/boot/dts/stih416-b2020-revE.dts
+++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
@@ -31,5 +31,10 @@
ethernet1: dwmac@fef08000 {
snps,reset-gpio = <&PIO0 7>;
};
+
+ miphy365x_phy: miphy365x@fe382000 {
+ st,pcie-tx-pol-inv;
+ st,sata-gen = <3>;
+ };
};
};
diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
index 4e2df66..6f1145c 100644
--- a/arch/arm/boot/dts/stih416-b2020.dts
+++ b/arch/arm/boot/dts/stih416-b2020.dts
@@ -12,4 +12,11 @@
/ {
model = "STiH416 B2020";
compatible = "st,stih416-b2020", "st,stih416";
+
+ soc {
+ miphy365x_phy: miphy365x@fe382000 {
+ st,pcie-tx-pol-inv;
+ st,sata-gen = <3>;
+ };
+ };
};
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 06473c5..6d7aee3 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -9,6 +9,8 @@
#include "stih41x.dtsi"
#include "stih416-clock.dtsi"
#include "stih416-pinctrl.dtsi"
+
+#include <dt-bindings/phy/phy-miphy365x.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/reset-controller/stih416-resets.h>
/ {
@@ -236,5 +238,24 @@
resets = <&powerdown STIH416_KEYSCAN_POWERDOWN>,
<&softreset STIH416_KEYSCAN_SOFTRESET>;
};
+
+ miphy365x_phy: miphy365x@fe382000 {
+ compatible = "st,miphy365x-phy";
+ st,syscfg = <&syscfg_rear>;
+ #phy-cells = <2>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ phy_port0: port@fe382000 {
+ reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
+ reg-names = "sata", "pcie";
+ };
+
+ phy_port1: port@fe38a000 {
+ reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
+ reg-names = "sata", "pcie";
+ };
+ };
};
};
--
1.8.3.2
The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
devices. It has 2 ports which it can use for either; both SATA, both
PCIe or one of each in any configuration.
Acked-by: Kishon Vijay Abraham I <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Signed-off-by: Alexandre Torgue <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
drivers/phy/Kconfig | 10 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 641 insertions(+)
create mode 100644 drivers/phy/phy-miphy365x.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 16a2f06..5d54dcf 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -31,6 +31,16 @@ config PHY_MVEBU_SATA
depends on OF
select GENERIC_PHY
+config PHY_MIPHY365X
+ tristate "STMicroelectronics MIPHY365X PHY driver for STiH41x series"
+ depends on ARCH_STI
+ depends on GENERIC_PHY
+ depends on HAS_IOMEM
+ depends on OF
+ help
+ Enable this to support the miphy transceiver (for SATA/PCIE)
+ that is part of STMicroelectronics STiH41x SoC series.
+
config OMAP_CONTROL_PHY
tristate "OMAP CONTROL PHY Driver"
depends on ARCH_OMAP2PLUS || COMPILE_TEST
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index b4f1d57..4602f67 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -7,6 +7,7 @@ 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
obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
+obj-$(CONFIG_PHY_MIPHY365X) += phy-miphy365x.o
obj-$(CONFIG_OMAP_CONTROL_PHY) += phy-omap-control.o
obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
obj-$(CONFIG_TI_PIPE3) += phy-ti-pipe3.o
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
new file mode 100644
index 0000000..1109f42
--- /dev/null
+++ b/drivers/phy/phy-miphy365x.c
@@ -0,0 +1,630 @@
+/*
+ * Copyright (C) 2014 STMicroelectronics
+ *
+ * STMicroelectronics PHY driver MiPHY365 (for SoC STiH416).
+ *
+ * Author: Alexandre Torgue <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/clk.h>
+#include <linux/phy/phy.h>
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/phy/phy-miphy365x.h>
+
+#define HFC_TIMEOUT 50
+
+#define SYSCFG_2521 0x824
+#define SYSCFG_2522 0x828
+#define SYSCFG_PCIE_SATA_MASK BIT(1)
+#define SYSCFG_PCIE_SATA_POS 1
+
+/* MiPHY365x register definitiona */
+#define RESET_REG 0x00
+#define RST_PLL BIT(1)
+#define RST_PLL_CAL BIT(2)
+#define RST_RX BIT(4)
+#define RST_MACRO BIT(7)
+
+#define STATUS_REG 0x01
+#define IDLL_RDY BIT(0)
+#define PLL_RDY BIT(1)
+#define DES_BIT_LOCK BIT(2)
+#define DES_SYMBOL_LOCK BIT(3)
+
+#define CTRL_REG 0x02
+#define TERM_EN BIT(0)
+#define PCI_EN BIT(2)
+#define DES_BIT_LOCK_EN BIT(3)
+#define TX_POL BIT(5)
+
+#define INT_CTRL_REG 0x03
+
+#define BOUNDARY1_REG 0x10
+#define SPDSEL_SEL BIT(0)
+
+#define BOUNDARY3_REG 0x12
+#define TX_SPDSEL_GEN1_VAL 0
+#define TX_SPDSEL_GEN2_VAL 0x01
+#define TX_SPDSEL_GEN3_VAL 0x02
+#define RX_SPDSEL_GEN1_VAL 0
+#define RX_SPDSEL_GEN2_VAL (0x01 << 3)
+#define RX_SPDSEL_GEN3_VAL (0x02 << 3)
+
+#define PCIE_REG 0x16
+
+#define BUF_SEL_REG 0x20
+#define CONF_GEN_SEL_GEN3 0x02
+#define CONF_GEN_SEL_GEN2 0x01
+#define PD_VDDTFILTER BIT(4)
+
+#define TXBUF1_REG 0x21
+#define SWING_VAL 0x04
+#define SWING_VAL_GEN1 0x03
+#define PREEMPH_VAL (0x3 << 5)
+
+#define TXBUF2_REG 0x22
+#define TXSLEW_VAL 0x2
+#define TXSLEW_VAL_GEN1 0x4
+
+#define RXBUF_OFFSET_CTRL_REG 0x23
+
+#define RXBUF_REG 0x25
+#define SDTHRES_VAL 0x01
+#define EQ_ON3 (0x03 << 4)
+#define EQ_ON1 (0x01 << 4)
+
+#define COMP_CTRL1_REG 0x40
+#define START_COMSR BIT(0)
+#define START_COMZC BIT(1)
+#define COMSR_DONE BIT(2)
+#define COMZC_DONE BIT(3)
+#define COMP_AUTO_LOAD BIT(4)
+
+#define COMP_CTRL2_REG 0x41
+#define COMP_2MHZ_RAT_GEN1 0x1e
+#define COMP_2MHZ_RAT 0xf
+
+#define COMP_CTRL3_REG 0x42
+#define COMSR_COMP_REF 0x33
+
+#define COMP_IDLL_REG 0x47
+#define COMZC_IDLL 0x2a
+
+#define PLL_CTRL1_REG 0x50
+#define PLL_START_CAL BIT(0)
+#define BUF_EN BIT(2)
+#define SYNCHRO_TX BIT(3)
+#define SSC_EN BIT(6)
+#define CONFIG_PLL BIT(7)
+
+#define PLL_CTRL2_REG 0x51
+#define BYPASS_PLL_CAL BIT(1)
+
+#define PLL_RAT_REG 0x52
+
+#define PLL_SSC_STEP_MSB_REG 0x56
+#define PLL_SSC_STEP_MSB_VAL 0x03
+
+#define PLL_SSC_STEP_LSB_REG 0x57
+#define PLL_SSC_STEP_LSB_VAL 0x63
+
+#define PLL_SSC_PER_MSB_REG 0x58
+#define PLL_SSC_PER_MSB_VAL 0
+
+#define PLL_SSC_PER_LSB_REG 0x59
+#define PLL_SSC_PER_LSB_VAL 0xf1
+
+#define IDLL_TEST_REG 0x72
+#define START_CLK_HF BIT(6)
+
+#define DES_BITLOCK_REG 0x86
+#define BIT_LOCK_LEVEL 0x01
+#define BIT_LOCK_CNT_512 (0x03 << 5)
+
+static u8 ports[] = { MIPHY_PORT_0, MIPHY_PORT_1 };
+
+struct miphy365x {
+ struct phy *phy;
+ void __iomem *base;
+ void __iomem *sata;
+ void __iomem *pcie;
+ u8 type;
+ u8 port;
+};
+
+struct miphy365x_dev {
+ struct device *dev;
+ struct mutex miphy_mutex;
+ struct miphy365x phys[ARRAY_SIZE(ports)];
+ bool pcie_tx_pol_inv;
+ bool sata_tx_pol_inv;
+ u32 sata_gen;
+ struct regmap *regmap;
+};
+
+/*
+ * These values are represented in Device tree. They are considered to be ABI
+ * and although they can be extended any existing values must not change.
+ */
+enum miphy_sata_gen {
+ SATA_GEN1 = 1,
+ SATA_GEN2,
+ SATA_GEN3
+};
+
+static u8 rx_tx_spd[] = {
+ TX_SPDSEL_GEN1_VAL | RX_SPDSEL_GEN1_VAL,
+ TX_SPDSEL_GEN2_VAL | RX_SPDSEL_GEN2_VAL,
+ TX_SPDSEL_GEN3_VAL | RX_SPDSEL_GEN3_VAL
+};
+
+#define miphy365x_phy_to_dev(inst) \
+ container_of((inst), struct miphy365x_dev, phys[(inst)->port]);
+
+/*
+ * This function selects the system configuration,
+ * either two SATA, one SATA and one PCIe, or two PCIe lanes.
+ */
+static int miphy365x_set_path(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ u8 config = miphy_phy->type | miphy_phy->port;
+ u32 mask = SYSCFG_PCIE_SATA_MASK;
+ u32 reg;
+ bool sata;
+
+ switch (config) {
+ case MIPHY_SATA_PORT0:
+ reg = SYSCFG_2521;
+ sata = true;
+ break;
+ case MIPHY_PCIE_PORT1:
+ reg = SYSCFG_2522;
+ sata = false;
+ break;
+ default:
+ dev_err(miphy_dev->dev, "Configuration not supported\n");
+ return -EINVAL;
+ }
+
+ return regmap_update_bits(miphy_dev->regmap, reg, mask,
+ sata << SYSCFG_PCIE_SATA_POS);
+}
+
+static void miphy365x_init_pcie_port(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ u8 val;
+
+ if (!miphy_dev->pcie_tx_pol_inv)
+ return;
+
+ /* Invert Tx polarity and clear pci_txdetect_pol bit */
+ val = TERM_EN | PCI_EN | DES_BIT_LOCK_EN | TX_POL;
+ writeb_relaxed(val, miphy_phy->base + CTRL_REG);
+ writeb_relaxed(0x00, miphy_phy->base + PCIE_REG);
+}
+
+static inline int miphy365x_hfc_not_rdy(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ int timeout = HFC_TIMEOUT;
+ u8 mask = IDLL_RDY | PLL_RDY;
+ u8 regval;
+
+ do {
+ regval = readb_relaxed(miphy_phy->base + STATUS_REG);
+ usleep_range(2000, 2500);
+ } while (timeout-- && (regval & mask));
+
+ if (timeout < 0) {
+ dev_err(miphy_dev->dev, "HFC ready timeout!\n");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static inline int miphy365x_rdy(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ int timeout = HFC_TIMEOUT;
+ u8 mask = IDLL_RDY | PLL_RDY;
+ u8 regval;
+
+ do {
+ regval = readb_relaxed(miphy_phy->base + STATUS_REG);
+ usleep_range(2000, 2500);
+ } while (timeout-- && ((regval & mask) != mask));
+
+ if (timeout < 0) {
+ dev_err(miphy_dev->dev, "PHY not ready timeout!\n");
+ return -EBUSY;
+ }
+
+ return 0;
+}
+
+static inline void miphy365x_set_comp(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ u8 val, mask;
+
+ if (miphy_dev->sata_gen == SATA_GEN1)
+ writeb_relaxed(COMP_2MHZ_RAT_GEN1,
+ miphy_phy->base + COMP_CTRL2_REG);
+ else
+ writeb_relaxed(COMP_2MHZ_RAT,
+ miphy_phy->base + COMP_CTRL2_REG);
+
+ if (miphy_dev->sata_gen != SATA_GEN3) {
+ writeb_relaxed(COMSR_COMP_REF,
+ miphy_phy->base + COMP_CTRL3_REG);
+ /*
+ * Force VCO current to value defined by address 0x5A
+ * and disable PCIe100Mref bit
+ * Enable auto load compensation for pll_i_bias
+ */
+ writeb_relaxed(BYPASS_PLL_CAL, miphy_phy->base + PLL_CTRL2_REG);
+ writeb_relaxed(COMZC_IDLL, miphy_phy->base + COMP_IDLL_REG);
+ }
+
+ /*
+ * Force restart compensation and enable auto load
+ * for Comzc_Tx, Comzc_Rx and Comsr on macro
+ */
+ val = START_COMSR | START_COMZC | COMP_AUTO_LOAD;
+ writeb_relaxed(val, miphy_phy->base + COMP_CTRL1_REG);
+
+ mask = COMSR_DONE | COMZC_DONE;
+ while ((readb_relaxed(miphy_phy->base + COMP_CTRL1_REG) & mask) != mask)
+ cpu_relax();
+}
+
+static inline void miphy365x_set_ssc(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ u8 val;
+
+ /*
+ * SSC Settings. SSC will be enabled through Link
+ * SSC Ampl. = 0.4%
+ * SSC Freq = 31KHz
+ */
+ writeb_relaxed(PLL_SSC_STEP_MSB_VAL,
+ miphy_phy->base + PLL_SSC_STEP_MSB_REG);
+ writeb_relaxed(PLL_SSC_STEP_LSB_VAL,
+ miphy_phy->base + PLL_SSC_STEP_LSB_REG);
+ writeb_relaxed(PLL_SSC_PER_MSB_VAL,
+ miphy_phy->base + PLL_SSC_PER_MSB_REG);
+ writeb_relaxed(PLL_SSC_PER_LSB_VAL,
+ miphy_phy->base + PLL_SSC_PER_LSB_REG);
+
+ /* SSC Settings complete */
+ if (miphy_dev->sata_gen == SATA_GEN1) {
+ val = PLL_START_CAL | BUF_EN | SYNCHRO_TX | CONFIG_PLL;
+ writeb_relaxed(val, miphy_phy->base + PLL_CTRL1_REG);
+ } else {
+ val = SSC_EN | PLL_START_CAL | BUF_EN | SYNCHRO_TX | CONFIG_PLL;
+ writeb_relaxed(val, miphy_phy->base + PLL_CTRL1_REG);
+ }
+}
+
+static int miphy365x_init_sata_port(struct miphy365x *miphy_phy,
+ struct miphy365x_dev *miphy_dev)
+{
+ int ret;
+ u8 val;
+
+ /*
+ * Force PHY macro reset, PLL calibration reset, PLL reset
+ * and assert Deserializer Reset
+ */
+ val = RST_PLL | RST_PLL_CAL | RST_RX | RST_MACRO;
+ writeb_relaxed(val, miphy_phy->base + RESET_REG);
+
+ if (miphy_dev->sata_tx_pol_inv)
+ writeb_relaxed(TX_POL, miphy_phy->base + CTRL_REG);
+
+ /*
+ * Force macro1 to use rx_lspd, tx_lspd
+ * Force Rx_Clock on first I-DLL phase
+ * Force Des in HP mode on macro, rx_lspd, tx_lspd for Gen2/3
+ */
+ writeb_relaxed(SPDSEL_SEL, miphy_phy->base + BOUNDARY1_REG);
+ writeb_relaxed(START_CLK_HF, miphy_phy->base + IDLL_TEST_REG);
+ val = rx_tx_spd[miphy_dev->sata_gen];
+ writeb_relaxed(val, miphy_phy->base + BOUNDARY3_REG);
+
+ /* Wait for HFC_READY = 0 */
+ ret = miphy365x_hfc_not_rdy(miphy_phy, miphy_dev);
+ if (ret)
+ return ret;
+
+ /* Compensation Recalibration */
+ miphy365x_set_comp(miphy_phy, miphy_dev);
+
+ switch (miphy_dev->sata_gen) {
+ case SATA_GEN3:
+ /*
+ * TX Swing target 550-600mv peak to peak diff
+ * Tx Slew target 90-110ps rising/falling time
+ * Rx Eq ON3, Sigdet threshold SDTH1
+ */
+ val = PD_VDDTFILTER | CONF_GEN_SEL_GEN3;
+ writeb_relaxed(val, miphy_phy->base + BUF_SEL_REG);
+ val = SWING_VAL | PREEMPH_VAL;
+ writeb_relaxed(val, miphy_phy->base + TXBUF1_REG);
+ writeb_relaxed(TXSLEW_VAL, miphy_phy->base + TXBUF2_REG);
+ writeb_relaxed(0x00, miphy_phy->base + RXBUF_OFFSET_CTRL_REG);
+ val = SDTHRES_VAL | EQ_ON3;
+ writeb_relaxed(val, miphy_phy->base + RXBUF_REG);
+ break;
+ case SATA_GEN2:
+ /*
+ * conf gen sel=0x1 to program Gen2 banked registers
+ * VDDT filter ON
+ * Tx Swing target 550-600mV peak-to-peak diff
+ * Tx Slew target 90-110 ps rising/falling time
+ * RX Equalization ON1, Sigdet threshold SDTH1
+ */
+ writeb_relaxed(CONF_GEN_SEL_GEN2,
+ miphy_phy->base + BUF_SEL_REG);
+ writeb_relaxed(SWING_VAL, miphy_phy->base + TXBUF1_REG);
+ writeb_relaxed(TXSLEW_VAL, miphy_phy->base + TXBUF2_REG);
+ val = SDTHRES_VAL | EQ_ON1;
+ writeb_relaxed(val, miphy_phy->base + RXBUF_REG);
+ break;
+ case SATA_GEN1:
+ /*
+ * conf gen sel = 00b to program Gen1 banked registers
+ * VDDT filter ON
+ * Tx Swing target 500-550mV peak-to-peak diff
+ * Tx Slew target120-140 ps rising/falling time
+ */
+ writeb_relaxed(PD_VDDTFILTER, miphy_phy->base + BUF_SEL_REG);
+ writeb_relaxed(SWING_VAL_GEN1, miphy_phy->base + TXBUF1_REG);
+ writeb_relaxed(TXSLEW_VAL_GEN1, miphy_phy->base + TXBUF2_REG);
+ break;
+ default:
+ break;
+ }
+
+ /* Force Macro1 in partial mode & release pll cal reset */
+ writeb_relaxed(RST_RX, miphy_phy->base + RESET_REG);
+ usleep_range(100, 150);
+
+ miphy365x_set_ssc(miphy_phy, miphy_dev);
+
+ /* Wait for phy_ready */
+ ret = miphy365x_rdy(miphy_phy, miphy_dev);
+ if (ret)
+ return ret;
+
+ /*
+ * Enable macro1 to use rx_lspd & tx_lspd
+ * Release Rx_Clock on first I-DLL phase on macro1
+ * Assert deserializer reset
+ * des_bit_lock_en is set
+ * bit lock detection strength
+ * Deassert deserializer reset
+ */
+ writeb_relaxed(0x00, miphy_phy->base + BOUNDARY1_REG);
+ writeb_relaxed(0x00, miphy_phy->base + IDLL_TEST_REG);
+ writeb_relaxed(RST_RX, miphy_phy->base + RESET_REG);
+ val = miphy_dev->sata_tx_pol_inv ?
+ (TX_POL | DES_BIT_LOCK_EN) : DES_BIT_LOCK_EN;
+ writeb_relaxed(val, miphy_phy->base + CTRL_REG);
+
+ val = BIT_LOCK_CNT_512 | BIT_LOCK_LEVEL;
+ writeb_relaxed(val, miphy_phy->base + DES_BITLOCK_REG);
+ writeb_relaxed(0x00, miphy_phy->base + RESET_REG);
+
+ return 0;
+}
+
+static int miphy365x_init(struct phy *phy)
+{
+ int ret = 0;
+ struct miphy365x *miphy_phy = phy_get_drvdata(phy);
+ struct miphy365x_dev *miphy_dev = miphy365x_phy_to_dev(miphy_phy);
+
+ mutex_lock(&miphy_dev->miphy_mutex);
+
+ ret = miphy365x_set_path(miphy_phy, miphy_dev);
+ if (ret) {
+ mutex_unlock(&miphy_dev->miphy_mutex);
+ return ret;
+ }
+
+ /* Initialise Miphy for PCIe or SATA */
+ if (miphy_phy->type == MIPHY_TYPE_PCIE)
+ miphy365x_init_pcie_port(miphy_phy, miphy_dev);
+ else
+ ret = miphy365x_init_sata_port(miphy_phy, miphy_dev);
+
+ mutex_unlock(&miphy_dev->miphy_mutex);
+
+ return ret;
+}
+
+static struct phy *miphy365x_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ struct miphy365x_dev *state = dev_get_drvdata(dev);
+ u8 port, type;
+
+ if (args->count != 2) {
+ dev_err(dev, "Invalid number of cells in 'phy' property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (args->args[0] & 0xFFFFFF00 || args->args[1] & 0xFFFFFF00) {
+ dev_err(dev, "Unsupported flags set in 'phy' property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ port = args->args[0];
+ type = args->args[1];
+
+ if (WARN_ON(port >= ARRAY_SIZE(ports)))
+ return ERR_PTR(-EINVAL);
+
+ if (type == MIPHY_TYPE_SATA)
+ state->phys[port].base = state->phys[port].sata;
+ else if (type == MIPHY_TYPE_PCIE)
+ state->phys[port].base = state->phys[port].pcie;
+ else {
+ WARN(1, "Invalid type specified in DT");
+ return ERR_PTR(-EINVAL);
+ }
+
+ state->phys[port].type = type;
+
+ return state->phys[port].phy;
+}
+
+static struct phy_ops miphy365x_ops = {
+ .init = miphy365x_init,
+ .owner = THIS_MODULE,
+};
+
+static int miphy365x_get_base_addr(struct platform_device *pdev,
+ struct miphy365x_phy *phy, u8 port)
+{
+ struct resource *res;
+ char type[6];
+
+ sprintf(type, "sata%d", port);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
+ if (!res)
+ return -ENODEV;
+
+ phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!phy->sata)
+ return -ENOMEM;
+
+ sprintf(type, "pcie%d", port);
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
+ if (!res)
+ return -ENODEV;
+
+ phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!phy->pcie)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int miphy365x_of_probe(struct device_node *np,
+ struct miphy365x_dev *phy_dev)
+{
+ phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+ if (IS_ERR(phy_dev->regmap)) {
+ dev_err(phy_dev->dev, "No syscfg phandle specified\n");
+ return PTR_ERR(phy_dev->regmap);
+ }
+
+ of_property_read_u32(np, "st,sata-gen", &phy_dev->sata_gen);
+ if (!phy_dev->sata_gen)
+ phy_dev->sata_gen = SATA_GEN1;
+
+ phy_dev->pcie_tx_pol_inv =
+ of_property_read_bool(np, "st,pcie-tx-pol-inv");
+
+ phy_dev->sata_tx_pol_inv =
+ of_property_read_bool(np, "st,sata-tx-pol-inv");
+
+ return 0;
+}
+
+static int miphy365x_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct miphy365x_dev *phy_dev;
+ struct device *dev = &pdev->dev;
+ struct phy_provider *provider;
+ u8 port;
+ int ret;
+
+ if (!np) {
+ dev_err(dev, "No DT node found\n");
+ return -EINVAL;
+ }
+
+ phy_dev = devm_kzalloc(dev, sizeof(*phy_dev), GFP_KERNEL);
+ if (!phy_dev)
+ return -ENOMEM;
+
+ ret = miphy365x_of_probe(np, phy_dev);
+ if (ret)
+ return ret;
+
+ phy_dev->dev = dev;
+
+ dev_set_drvdata(dev, phy_dev);
+
+ mutex_init(&phy_dev->miphy_mutex);
+
+ for (port = 0; port < ARRAY_SIZE(ports); port++) {
+ struct phy *phy;
+
+ phy = devm_phy_create(dev, &miphy365x_ops, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "failed to create PHY on port %d\n", port);
+ return PTR_ERR(phy);
+ }
+
+ phy_dev->phys[port].phy = phy;
+ phy_dev->phys[port].port = port;
+
+ ret = miphy365x_get_base_addr(pdev,
+ &phy_dev->phys[port], port);
+ if (ret)
+ return ret;
+
+ phy_set_drvdata(phy, &phy_dev->phys[port]);
+ }
+
+ provider = devm_of_phy_provider_register(dev, miphy365x_xlate);
+ if (IS_ERR(provider))
+ return PTR_ERR(provider);
+
+ return 0;
+}
+
+static const struct of_device_id miphy365x_of_match[] = {
+ { .compatible = "st,miphy365x-phy", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, miphy365x_of_match);
+
+static struct platform_driver miphy365x_driver = {
+ .probe = miphy365x_probe,
+ .driver = {
+ .name = "miphy365x-phy",
+ .owner = THIS_MODULE,
+ .of_match_table = miphy365x_of_match,
+ }
+};
+module_platform_driver(miphy365x_driver);
+
+MODULE_AUTHOR("Alexandre Torgue <[email protected]>");
+MODULE_DESCRIPTION("STMicroelectronics miphy365x driver");
+MODULE_LICENSE("GPL v2");
--
1.8.3.2
---
drivers/phy/phy-miphy365x.c | 104 ++++++++++++++++++++++++++++++++------------
1 file changed, 75 insertions(+), 29 deletions(-)
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
index 1109f42..2c4ea6e 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/of_address.h>
#include <linux/clk.h>
#include <linux/phy/phy.h>
#include <linux/delay.h>
@@ -144,6 +145,7 @@ struct miphy365x {
void __iomem *pcie;
u8 type;
u8 port;
+ bool enabled;
};
struct miphy365x_dev {
@@ -468,7 +470,7 @@ static struct phy *miphy365x_xlate(struct device *dev,
struct miphy365x_dev *state = dev_get_drvdata(dev);
u8 port, type;
- if (args->count != 2) {
+ if (args->args_count != 2) {
dev_err(dev, "Invalid number of cells in 'phy' property\n");
return ERR_PTR(-EINVAL);
}
@@ -484,6 +486,11 @@ static struct phy *miphy365x_xlate(struct device *dev,
if (WARN_ON(port >= ARRAY_SIZE(ports)))
return ERR_PTR(-EINVAL);
+ if (!state->phys[port].enabled) {
+ dev_warn(dev, "PHY port %d is disabled\n", port);
+ return ERR_PTR(-EINVAL);
+ }
+
if (type == MIPHY_TYPE_SATA)
state->phys[port].base = state->phys[port].sata;
else if (type == MIPHY_TYPE_PCIE)
@@ -503,38 +510,75 @@ static struct phy_ops miphy365x_ops = {
.owner = THIS_MODULE,
};
-static int miphy365x_get_base_addr(struct platform_device *pdev,
- struct miphy365x_phy *phy, u8 port)
+static int miphy365x_get_base_addr_one(struct platform_device *pdev,
+ struct miphy365x *phy,
+ struct device_node *child,
+ int index)
{
- struct resource *res;
- char type[6];
+ void __iomem *base;
+ const char *name;
+ int ret;
- sprintf(type, "sata%d", port);
+ base = of_iomap(child, index);
+ if (!base) {
+ dev_err(&pdev->dev, "Failed to map %s\n", child->full_name);
+ return -EINVAL;
+ }
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
- if (!res)
- return -ENODEV;
+ ret = of_property_read_string_index(child, "reg-names", index, &name);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "%s: no reg-names property not found\n", child->name);
+ return ret;
+ }
- phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!phy->sata)
- return -ENOMEM;
+ if (!strncmp(name, "sata", 4))
+ phy->sata = base;
+ else if (!strncmp(name, "pcie", 4))
+ phy->pcie = base;
+ else {
+ dev_err(&pdev->dev, "reg-names in %s not sata or pcie: %s",
+ child->name, name);
+ return -EINVAL;
+ }
- sprintf(type, "pcie%d", port);
+ return 0;
+}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
- if (!res)
- return -ENODEV;
+static int miphy365x_get_base_addr(struct platform_device *pdev,
+ struct miphy365x *phy,
+ struct device_node *child)
+{
+ int index;
+ int ret;
- phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!phy->pcie)
- return -ENOMEM;
+ /* Each port handles SATA or PCIE. */
+ for (index = 0; index < 2; index++) {
+ ret = miphy365x_get_base_addr_one(pdev, phy,
+ child, index);
+ if (ret)
+ return ret;
+ }
return 0;
}
-static int miphy365x_of_probe(struct device_node *np,
+static int miphy365x_of_probe(struct platform_device *pdev,
struct miphy365x_dev *phy_dev)
{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *child;
+ int child_count = 0;
+
+ for_each_child_of_node(np, child)
+ child_count++;
+
+ if (child_count != ARRAY_SIZE(ports)) {
+ dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
+ ARRAY_SIZE(ports), child_count);
+ return -EINVAL;
+ }
+
phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
if (IS_ERR(phy_dev->regmap)) {
dev_err(phy_dev->dev, "No syscfg phandle specified\n");
@@ -556,11 +600,10 @@ static int miphy365x_of_probe(struct device_node *np,
static int miphy365x_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *child, *np = pdev->dev.of_node;
struct miphy365x_dev *phy_dev;
struct device *dev = &pdev->dev;
struct phy_provider *provider;
- u8 port;
int ret;
if (!np) {
@@ -572,7 +615,7 @@ static int miphy365x_probe(struct platform_device *pdev)
if (!phy_dev)
return -ENOMEM;
- ret = miphy365x_of_probe(np, phy_dev);
+ ret = miphy365x_of_probe(pdev, phy_dev);
if (ret)
return ret;
@@ -582,8 +625,9 @@ static int miphy365x_probe(struct platform_device *pdev)
mutex_init(&phy_dev->miphy_mutex);
- for (port = 0; port < ARRAY_SIZE(ports); port++) {
+ for_each_child_of_node(np, child) {
struct phy *phy;
+ static int port = 0;
phy = devm_phy_create(dev, &miphy365x_ops, NULL);
if (IS_ERR(phy)) {
@@ -591,15 +635,17 @@ static int miphy365x_probe(struct platform_device *pdev)
return PTR_ERR(phy);
}
- phy_dev->phys[port].phy = phy;
- phy_dev->phys[port].port = port;
-
- ret = miphy365x_get_base_addr(pdev,
- &phy_dev->phys[port], port);
+ ret = miphy365x_get_base_addr(pdev, &phy_dev->phys[port],
+ child);
if (ret)
return ret;
+ phy_dev->phys[port].phy = phy;
+ phy_dev->phys[port].port = port;
+ phy_dev->phys[port].enabled = !!of_device_is_available(child);
+
phy_set_drvdata(phy, &phy_dev->phys[port]);
+ port++;
}
provider = devm_of_phy_provider_register(dev, miphy365x_xlate);
--
1.8.3.2
This provides the shared header file which will be reference from both
the MiPHY365x driver and its associated Device Tree node(s).
Cc: Kishon Vijay Abraham I <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Acked-by: Alexandre Torgue <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
---
include/dt-bindings/phy/phy-miphy365x.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 include/dt-bindings/phy/phy-miphy365x.h
diff --git a/include/dt-bindings/phy/phy-miphy365x.h b/include/dt-bindings/phy/phy-miphy365x.h
new file mode 100644
index 0000000..8757c02
--- /dev/null
+++ b/include/dt-bindings/phy/phy-miphy365x.h
@@ -0,0 +1,25 @@
+/*
+ * This header provides constants for the phy framework
+ * based on the STMicroelectronics miphy365x.
+ */
+#ifndef _DT_BINDINGS_PHY_MIPHY
+#define _DT_BINDINGS_PHY_MIPHY
+
+/* Supports 16 ports without a datatype change (u8 & 0xF0). */
+#define MIPHY_PORT_0 0
+#define MIPHY_PORT_1 1
+#define MIPHY_PORT_2 2
+#define MIPHY_PORT_3 3
+
+/* Supports 16 types without a datatype change (u8 & 0x0F). */
+#define MIPHY_TYPE_SHIFT 4
+#define MIPHY_TYPE_SATA (0 << MIPHY_TYPE_SHIFT)
+#define MIPHY_TYPE_PCIE (1 << MIPHY_TYPE_SHIFT)
+#define MIPHY_TYPE_USB (2 << MIPHY_TYPE_SHIFT)
+
+#define MIPHY_SATA_PORT0 (MIPHY_TYPE_SATA) | (MIPHY_PORT_0)
+#define MIPHY_SATA_PORT1 (MIPHY_TYPE_SATA) | (MIPHY_PORT_1)
+#define MIPHY_PCIE_PORT0 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_0)
+#define MIPHY_PCIE_PORT1 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_1)
+
+#endif /* _DT_BINDINGS_PHY_MIPHY */
--
1.8.3.2
Hi Lee,
No description, no sign-off?
On 06/30/2014 03:01 PM, Lee Jones wrote:
> ---
> drivers/phy/phy-miphy365x.c | 104 ++++++++++++++++++++++++++++++++------------
> 1 file changed, 75 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> index 1109f42..2c4ea6e 100644
> --- a/drivers/phy/phy-miphy365x.c
> +++ b/drivers/phy/phy-miphy365x.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/of_address.h>
> #include <linux/clk.h>
> #include <linux/phy/phy.h>
> #include <linux/delay.h>
> @@ -144,6 +145,7 @@ struct miphy365x {
> void __iomem *pcie;
> u8 type;
> u8 port;
> + bool enabled;
> };
>
> struct miphy365x_dev {
> @@ -468,7 +470,7 @@ static struct phy *miphy365x_xlate(struct device *dev,
> struct miphy365x_dev *state = dev_get_drvdata(dev);
> u8 port, type;
>
> - if (args->count != 2) {
> + if (args->args_count != 2) {
> dev_err(dev, "Invalid number of cells in 'phy' property\n");
> return ERR_PTR(-EINVAL);
> }
> @@ -484,6 +486,11 @@ static struct phy *miphy365x_xlate(struct device *dev,
> if (WARN_ON(port >= ARRAY_SIZE(ports)))
> return ERR_PTR(-EINVAL);
>
> + if (!state->phys[port].enabled) {
> + dev_warn(dev, "PHY port %d is disabled\n", port);
> + return ERR_PTR(-EINVAL);
> + }
> +
> if (type == MIPHY_TYPE_SATA)
> state->phys[port].base = state->phys[port].sata;
> else if (type == MIPHY_TYPE_PCIE)
> @@ -503,38 +510,75 @@ static struct phy_ops miphy365x_ops = {
> .owner = THIS_MODULE,
> };
>
> -static int miphy365x_get_base_addr(struct platform_device *pdev,
> - struct miphy365x_phy *phy, u8 port)
> +static int miphy365x_get_base_addr_one(struct platform_device *pdev,
> + struct miphy365x *phy,
> + struct device_node *child,
> + int index)
> {
> - struct resource *res;
> - char type[6];
> + void __iomem *base;
> + const char *name;
> + int ret;
>
> - sprintf(type, "sata%d", port);
> + base = of_iomap(child, index);
> + if (!base) {
> + dev_err(&pdev->dev, "Failed to map %s\n", child->full_name);
> + return -EINVAL;
> + }
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> - if (!res)
> - return -ENODEV;
> + ret = of_property_read_string_index(child, "reg-names", index, &name);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "%s: no reg-names property not found\n", child->name);
> + return ret;
> + }
>
> - phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!phy->sata)
> - return -ENOMEM;
> + if (!strncmp(name, "sata", 4))
> + phy->sata = base;
> + else if (!strncmp(name, "pcie", 4))
> + phy->pcie = base;
> + else {
> + dev_err(&pdev->dev, "reg-names in %s not sata or pcie: %s",
> + child->name, name);
> + return -EINVAL;
> + }
>
> - sprintf(type, "pcie%d", port);
> + return 0;
> +}
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> - if (!res)
> - return -ENODEV;
> +static int miphy365x_get_base_addr(struct platform_device *pdev,
> + struct miphy365x *phy,
> + struct device_node *child)
> +{
> + int index;
> + int ret;
>
> - phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!phy->pcie)
> - return -ENOMEM;
> + /* Each port handles SATA or PCIE. */
> + for (index = 0; index < 2; index++) {
> + ret = miphy365x_get_base_addr_one(pdev, phy,
> + child, index);
> + if (ret)
> + return ret;
> + }
>
> return 0;
> }
>
> -static int miphy365x_of_probe(struct device_node *np,
> +static int miphy365x_of_probe(struct platform_device *pdev,
> struct miphy365x_dev *phy_dev)
> {
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *child;
> + int child_count = 0;
> +
> + for_each_child_of_node(np, child)
> + child_count++;
> +
> + if (child_count != ARRAY_SIZE(ports)) {
> + dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
> + ARRAY_SIZE(ports), child_count);
> + return -EINVAL;
> + }
> +
> phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> if (IS_ERR(phy_dev->regmap)) {
> dev_err(phy_dev->dev, "No syscfg phandle specified\n");
> @@ -556,11 +600,10 @@ static int miphy365x_of_probe(struct device_node *np,
>
> static int miphy365x_probe(struct platform_device *pdev)
> {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *child, *np = pdev->dev.of_node;
> struct miphy365x_dev *phy_dev;
> struct device *dev = &pdev->dev;
> struct phy_provider *provider;
> - u8 port;
> int ret;
>
> if (!np) {
> @@ -572,7 +615,7 @@ static int miphy365x_probe(struct platform_device *pdev)
> if (!phy_dev)
> return -ENOMEM;
>
> - ret = miphy365x_of_probe(np, phy_dev);
> + ret = miphy365x_of_probe(pdev, phy_dev);
> if (ret)
> return ret;
>
> @@ -582,8 +625,9 @@ static int miphy365x_probe(struct platform_device *pdev)
>
> mutex_init(&phy_dev->miphy_mutex);
>
> - for (port = 0; port < ARRAY_SIZE(ports); port++) {
> + for_each_child_of_node(np, child) {
> struct phy *phy;
> + static int port = 0;
>
> phy = devm_phy_create(dev, &miphy365x_ops, NULL);
> if (IS_ERR(phy)) {
> @@ -591,15 +635,17 @@ static int miphy365x_probe(struct platform_device *pdev)
> return PTR_ERR(phy);
> }
>
> - phy_dev->phys[port].phy = phy;
> - phy_dev->phys[port].port = port;
> -
> - ret = miphy365x_get_base_addr(pdev,
> - &phy_dev->phys[port], port);
> + ret = miphy365x_get_base_addr(pdev, &phy_dev->phys[port],
> + child);
> if (ret)
> return ret;
>
> + phy_dev->phys[port].phy = phy;
> + phy_dev->phys[port].port = port;
> + phy_dev->phys[port].enabled = !!of_device_is_available(child);
> +
> phy_set_drvdata(phy, &phy_dev->phys[port]);
> + port++;
> }
>
> provider = devm_of_phy_provider_register(dev, miphy365x_xlate);
>
> No description, no sign-off?
Ah bum. More haste, less speed. Will fix.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
phy: miphy365x: Represent each PHY channel as a DT subnode
This has the added advantages of being able to enable/disable each
of the channels as simply as enabling/disabling the DT node.
Suggested-by: Kishon Vijay Abraham I <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
index 1109f42..2c4ea6e 100644
--- a/drivers/phy/phy-miphy365x.c
+++ b/drivers/phy/phy-miphy365x.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/of_address.h>
#include <linux/clk.h>
#include <linux/phy/phy.h>
#include <linux/delay.h>
@@ -144,6 +145,7 @@ struct miphy365x {
void __iomem *pcie;
u8 type;
u8 port;
+ bool enabled;
};
struct miphy365x_dev {
@@ -468,7 +470,7 @@ static struct phy *miphy365x_xlate(struct device *dev,
struct miphy365x_dev *state = dev_get_drvdata(dev);
u8 port, type;
- if (args->count != 2) {
+ if (args->args_count != 2) {
dev_err(dev, "Invalid number of cells in 'phy' property\n");
return ERR_PTR(-EINVAL);
}
@@ -484,6 +486,11 @@ static struct phy *miphy365x_xlate(struct device *dev,
if (WARN_ON(port >= ARRAY_SIZE(ports)))
return ERR_PTR(-EINVAL);
+ if (!state->phys[port].enabled) {
+ dev_warn(dev, "PHY port %d is disabled\n", port);
+ return ERR_PTR(-EINVAL);
+ }
+
if (type == MIPHY_TYPE_SATA)
state->phys[port].base = state->phys[port].sata;
else if (type == MIPHY_TYPE_PCIE)
@@ -503,38 +510,75 @@ static struct phy_ops miphy365x_ops = {
.owner = THIS_MODULE,
};
-static int miphy365x_get_base_addr(struct platform_device *pdev,
- struct miphy365x_phy *phy, u8 port)
+static int miphy365x_get_base_addr_one(struct platform_device *pdev,
+ struct miphy365x *phy,
+ struct device_node *child,
+ int index)
{
- struct resource *res;
- char type[6];
+ void __iomem *base;
+ const char *name;
+ int ret;
- sprintf(type, "sata%d", port);
+ base = of_iomap(child, index);
+ if (!base) {
+ dev_err(&pdev->dev, "Failed to map %s\n", child->full_name);
+ return -EINVAL;
+ }
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
- if (!res)
- return -ENODEV;
+ ret = of_property_read_string_index(child, "reg-names", index, &name);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "%s: no reg-names property not found\n", child->name);
+ return ret;
+ }
- phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!phy->sata)
- return -ENOMEM;
+ if (!strncmp(name, "sata", 4))
+ phy->sata = base;
+ else if (!strncmp(name, "pcie", 4))
+ phy->pcie = base;
+ else {
+ dev_err(&pdev->dev, "reg-names in %s not sata or pcie: %s",
+ child->name, name);
+ return -EINVAL;
+ }
- sprintf(type, "pcie%d", port);
+ return 0;
+}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
- if (!res)
- return -ENODEV;
+static int miphy365x_get_base_addr(struct platform_device *pdev,
+ struct miphy365x *phy,
+ struct device_node *child)
+{
+ int index;
+ int ret;
- phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!phy->pcie)
- return -ENOMEM;
+ /* Each port handles SATA or PCIE. */
+ for (index = 0; index < 2; index++) {
+ ret = miphy365x_get_base_addr_one(pdev, phy,
+ child, index);
+ if (ret)
+ return ret;
+ }
return 0;
}
-static int miphy365x_of_probe(struct device_node *np,
+static int miphy365x_of_probe(struct platform_device *pdev,
struct miphy365x_dev *phy_dev)
{
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *child;
+ int child_count = 0;
+
+ for_each_child_of_node(np, child)
+ child_count++;
+
+ if (child_count != ARRAY_SIZE(ports)) {
+ dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
+ ARRAY_SIZE(ports), child_count);
+ return -EINVAL;
+ }
+
phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
if (IS_ERR(phy_dev->regmap)) {
dev_err(phy_dev->dev, "No syscfg phandle specified\n");
@@ -556,11 +600,10 @@ static int miphy365x_of_probe(struct device_node *np,
static int miphy365x_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct device_node *child, *np = pdev->dev.of_node;
struct miphy365x_dev *phy_dev;
struct device *dev = &pdev->dev;
struct phy_provider *provider;
- u8 port;
int ret;
if (!np) {
@@ -572,7 +615,7 @@ static int miphy365x_probe(struct platform_device *pdev)
if (!phy_dev)
return -ENOMEM;
- ret = miphy365x_of_probe(np, phy_dev);
+ ret = miphy365x_of_probe(pdev, phy_dev);
if (ret)
return ret;
@@ -582,8 +625,9 @@ static int miphy365x_probe(struct platform_device *pdev)
mutex_init(&phy_dev->miphy_mutex);
- for (port = 0; port < ARRAY_SIZE(ports); port++) {
+ for_each_child_of_node(np, child) {
struct phy *phy;
+ static int port = 0;
phy = devm_phy_create(dev, &miphy365x_ops, NULL);
if (IS_ERR(phy)) {
@@ -591,15 +635,17 @@ static int miphy365x_probe(struct platform_device *pdev)
return PTR_ERR(phy);
}
- phy_dev->phys[port].phy = phy;
- phy_dev->phys[port].port = port;
-
- ret = miphy365x_get_base_addr(pdev,
- &phy_dev->phys[port], port);
+ ret = miphy365x_get_base_addr(pdev, &phy_dev->phys[port],
+ child);
if (ret)
return ret;
+ phy_dev->phys[port].phy = phy;
+ phy_dev->phys[port].port = port;
+ phy_dev->phys[port].enabled = !!of_device_is_available(child);
+
phy_set_drvdata(phy, &phy_dev->phys[port]);
+ port++;
}
provider = devm_of_phy_provider_register(dev, miphy365x_xlate);
On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> Hi Kishon,
>
> This submission is the same as the last one, only each channel is
> now represented by its own sub-node. The documentation has also
really cool.
Thanks
Kishon
> been updated accordingly.
>
> Kind regards,
> Lee
>
> .../devicetree/bindings/phy/phy-miphy365x.txt | 76 +++
> arch/arm/boot/dts/stih416-b2020-revE.dts | 5 +
> arch/arm/boot/dts/stih416-b2020.dts | 7 +
> arch/arm/boot/dts/stih416.dtsi | 21 +
> drivers/phy/Kconfig | 10 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-miphy365x.c | 676 +++++++++++++++++++++
> include/dt-bindings/phy/phy-miphy365x.h | 25 +
> 8 files changed, 821 insertions(+)
>
Hi,
On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> devices. It has 2 ports which it can use for either; both SATA, both
> PCIe or one of each in any configuration.
>
> Cc: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> Acked-by: Alexandre Torgue <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> .../devicetree/bindings/phy/phy-miphy365x.txt | 76 ++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> new file mode 100644
> index 0000000..d75f300
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> @@ -0,0 +1,76 @@
> +STMicroelectronics STi MIPHY365x PHY binding
> +============================================
> +
> +This binding describes a miphy device that is used to control PHY hardware
> +for SATA and PCIe.
> +
> +Required properties:
> +- compatible : Should be "st,miphy365x-phy"
> +- #phy-cells : Should be 2 (See second example)
> + First cell is the port number from:
> + - MIPHY_PORT_0
> + - MIPHY_PORT_1
I'm just thinking if we can directly give phandle to the sub-node
(channel0/channel1 or port0/port1) we won't need this information in the PHY
specifier. This might need some modification in the phy-core but that can be done.
> + Second cell is device type from:
> + - MIPHY_TYPE_SATA
> + - MIPHY_TYPE_PCI
> +- reg : Address and length of register sets for each device in
> + "reg-names"
> +- reg-names : The names of the register addresses corresponding to the
> + registers filled in "reg", from:
> + - sata0: For SATA port 0 registers
> + - sata1: For SATA port 1 registers
> + - pcie0: For PCIE port 0 registers
> + - pcie1: For PCIE port 1 registers
this information should be in the documentation of sub-nodes.
Cheers
Kishon
Hi,
On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> This provides the shared header file which will be reference from both
> the MiPHY365x driver and its associated Device Tree node(s).
>
> Cc: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> Acked-by: Alexandre Torgue <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> include/dt-bindings/phy/phy-miphy365x.h | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 include/dt-bindings/phy/phy-miphy365x.h
>
> diff --git a/include/dt-bindings/phy/phy-miphy365x.h b/include/dt-bindings/phy/phy-miphy365x.h
> new file mode 100644
> index 0000000..8757c02
> --- /dev/null
> +++ b/include/dt-bindings/phy/phy-miphy365x.h
> @@ -0,0 +1,25 @@
> +/*
> + * This header provides constants for the phy framework
> + * based on the STMicroelectronics miphy365x.
> + */
> +#ifndef _DT_BINDINGS_PHY_MIPHY
> +#define _DT_BINDINGS_PHY_MIPHY
> +
> +/* Supports 16 ports without a datatype change (u8 & 0xF0). */
> +#define MIPHY_PORT_0 0
> +#define MIPHY_PORT_1 1
> +#define MIPHY_PORT_2 2
> +#define MIPHY_PORT_3 3
> +
> +/* Supports 16 types without a datatype change (u8 & 0x0F). */
> +#define MIPHY_TYPE_SHIFT 4
> +#define MIPHY_TYPE_SATA (0 << MIPHY_TYPE_SHIFT)
> +#define MIPHY_TYPE_PCIE (1 << MIPHY_TYPE_SHIFT)
> +#define MIPHY_TYPE_USB (2 << MIPHY_TYPE_SHIFT)
> +
> +#define MIPHY_SATA_PORT0 (MIPHY_TYPE_SATA) | (MIPHY_PORT_0)
> +#define MIPHY_SATA_PORT1 (MIPHY_TYPE_SATA) | (MIPHY_PORT_1)
> +#define MIPHY_PCIE_PORT0 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_0)
> +#define MIPHY_PCIE_PORT1 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_1)
As Sergei pointed in your previous version it should be (X | Y).
Thanks
Kishon
Hi,
On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> devices. It has 2 ports which it can use for either; both SATA, both
> PCIe or one of each in any configuration.
>
> Acked-by: Kishon Vijay Abraham I <[email protected]>
Not yet..
> Acked-by: Mark Rutland <[email protected]>
> Signed-off-by: Alexandre Torgue <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/phy/Kconfig | 10 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 641 insertions(+)
> create mode 100644 drivers/phy/phy-miphy365x.c
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 16a2f06..5d54dcf 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -31,6 +31,16 @@ config PHY_MVEBU_SATA
> depends on OF
> select GENERIC_PHY
>
> +config PHY_MIPHY365X
> + tristate "STMicroelectronics MIPHY365X PHY driver for STiH41x series"
> + depends on ARCH_STI
> + depends on GENERIC_PHY
> + depends on HAS_IOMEM
> + depends on OF
> + help
> + Enable this to support the miphy transceiver (for SATA/PCIE)
> + that is part of STMicroelectronics STiH41x SoC series.
> +
> config OMAP_CONTROL_PHY
> tristate "OMAP CONTROL PHY Driver"
> depends on ARCH_OMAP2PLUS || COMPILE_TEST
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index b4f1d57..4602f67 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,6 +7,7 @@ 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
> obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o
> +obj-$(CONFIG_PHY_MIPHY365X) += phy-miphy365x.o
> obj-$(CONFIG_OMAP_CONTROL_PHY) += phy-omap-control.o
> obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
> obj-$(CONFIG_TI_PIPE3) += phy-ti-pipe3.o
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> new file mode 100644
> index 0000000..1109f42
> --- /dev/null
> +++ b/drivers/phy/phy-miphy365x.c
> @@ -0,0 +1,630 @@
> +/*
> + * Copyright (C) 2014 STMicroelectronics
> + *
> + * STMicroelectronics PHY driver MiPHY365 (for SoC STiH416).
> + *
> + * Author: Alexandre Torgue <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +#include <linux/phy/phy.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/phy/phy-miphy365x.h>
> +
> +#define HFC_TIMEOUT 50
> +
> +#define SYSCFG_2521 0x824
> +#define SYSCFG_2522 0x828
> +#define SYSCFG_PCIE_SATA_MASK BIT(1)
> +#define SYSCFG_PCIE_SATA_POS 1
> +
.
.
<snip>
.
.
> +
> +static u8 ports[] = { MIPHY_PORT_0, MIPHY_PORT_1 };
> +
> +struct miphy365x {
> + struct phy *phy;
> + void __iomem *base;
> + void __iomem *sata;
> + void __iomem *pcie;
> + u8 type;
> + u8 port;
> +};
> +
> +struct miphy365x_dev {
> + struct device *dev;
> + struct mutex miphy_mutex;
> + struct miphy365x phys[ARRAY_SIZE(ports)];
Avoid using fixed array sizes for ports or channels. Refer [1].
> + bool pcie_tx_pol_inv;
> + bool sata_tx_pol_inv;
> + u32 sata_gen;
> + struct regmap *regmap;
> +};
> +
> +/*
> + * These values are represented in Device tree. They are considered to be ABI
> + * and although they can be extended any existing values must not change.
> + */
> +enum miphy_sata_gen {
> + SATA_GEN1 = 1,
> + SATA_GEN2,
> + SATA_GEN3
> +};
> +
> +static u8 rx_tx_spd[] = {
> + TX_SPDSEL_GEN1_VAL | RX_SPDSEL_GEN1_VAL,
> + TX_SPDSEL_GEN2_VAL | RX_SPDSEL_GEN2_VAL,
> + TX_SPDSEL_GEN3_VAL | RX_SPDSEL_GEN3_VAL
> +};
> +
> +#define miphy365x_phy_to_dev(inst) \
> + container_of((inst), struct miphy365x_dev, phys[(inst)->port]);
You can also get rid of this and use dev_get_drvdata(phy->dev->parent);
> +
> +/*
> + * This function selects the system configuration,
> + * either two SATA, one SATA and one PCIe, or two PCIe lanes.
> + */
> +static int miphy365x_set_path(struct miphy365x *miphy_phy,
> + struct miphy365x_dev *miphy_dev)
> +{
> + u8 config = miphy_phy->type | miphy_phy->port;
> + u32 mask = SYSCFG_PCIE_SATA_MASK;
> + u32 reg;
> + bool sata;
> +
> + switch (config) {
> + case MIPHY_SATA_PORT0:
> + reg = SYSCFG_2521;
> + sata = true;
> + break;
> + case MIPHY_PCIE_PORT1:
> + reg = SYSCFG_2522;
> + sata = false;
> + break;
> + default:
> + dev_err(miphy_dev->dev, "Configuration not supported\n");
> + return -EINVAL;
> + }
> +
> + return regmap_update_bits(miphy_dev->regmap, reg, mask,
> + sata << SYSCFG_PCIE_SATA_POS);
> +}
> +
> +static void miphy365x_init_pcie_port(struct miphy365x *miphy_phy,
> + struct miphy365x_dev *miphy_dev)
I would have this return int so that it's API is similar to that of sata.
> +{
> + u8 val;
> +
> + if (!miphy_dev->pcie_tx_pol_inv)
> + return;
> +
> + /* Invert Tx polarity and clear pci_txdetect_pol bit */
> + val = TERM_EN | PCI_EN | DES_BIT_LOCK_EN | TX_POL;
> + writeb_relaxed(val, miphy_phy->base + CTRL_REG);
> + writeb_relaxed(0x00, miphy_phy->base + PCIE_REG);
> +}
> +
> +static inline int miphy365x_hfc_not_rdy(struct miphy365x *miphy_phy,
> + struct miphy365x_dev *miphy_dev)
> +{
> + int timeout = HFC_TIMEOUT;
> + u8 mask = IDLL_RDY | PLL_RDY;
> + u8 regval;
> +
> + do {
> + regval = readb_relaxed(miphy_phy->base + STATUS_REG);
> + usleep_range(2000, 2500);
msleep(2)?
> + } while (timeout-- && (regval & mask));
Instead use timeout using jiffies?
> +
> + if (timeout < 0) {
> + dev_err(miphy_dev->dev, "HFC ready timeout!\n");
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +static inline int miphy365x_rdy(struct miphy365x *miphy_phy,
> + struct miphy365x_dev *miphy_dev)
> +{
> + int timeout = HFC_TIMEOUT;
> + u8 mask = IDLL_RDY | PLL_RDY;
> + u8 regval;
> +
> + do {
> + regval = readb_relaxed(miphy_phy->base + STATUS_REG);
> + usleep_range(2000, 2500);
> + } while (timeout-- && ((regval & mask) != mask));
same here.
> +
> + if (timeout < 0) {
> + dev_err(miphy_dev->dev, "PHY not ready timeout!\n");
> + return -EBUSY;
> + }
> +
> + return 0;
> +}
> +
> +static inline void miphy365x_set_comp(struct miphy365x *miphy_phy,
> + struct miphy365x_dev *miphy_dev)
> +{
> + u8 val, mask;
> +
> + if (miphy_dev->sata_gen == SATA_GEN1)
> + writeb_relaxed(COMP_2MHZ_RAT_GEN1,
> + miphy_phy->base + COMP_CTRL2_REG);
> + else
> + writeb_relaxed(COMP_2MHZ_RAT,
> + miphy_phy->base + COMP_CTRL2_REG);
Btw don't you think it will be safe to use readb/writeb instead of
readb_relaxed/writeb_relaxed here and everywhere else?
> +
> + if (miphy_dev->sata_gen != SATA_GEN3) {
> + writeb_relaxed(COMSR_COMP_REF,
> + miphy_phy->base + COMP_CTRL3_REG);
> + /*
> + * Force VCO current to value defined by address 0x5A
> + * and disable PCIe100Mref bit
> + * Enable auto load compensation for pll_i_bias
> + */
> + writeb_relaxed(BYPASS_PLL_CAL, miphy_phy->base + PLL_CTRL2_REG);
> + writeb_relaxed(COMZC_IDLL, miphy_phy->base + COMP_IDLL_REG);
> + }
> +
.
.
<snip>
.
.
> +
> +static int miphy365x_init(struct phy *phy)
> +{
> + int ret = 0;
> + struct miphy365x *miphy_phy = phy_get_drvdata(phy);
> + struct miphy365x_dev *miphy_dev = miphy365x_phy_to_dev(miphy_phy);
> +
> + mutex_lock(&miphy_dev->miphy_mutex);
> +
> + ret = miphy365x_set_path(miphy_phy, miphy_dev);
> + if (ret) {
> + mutex_unlock(&miphy_dev->miphy_mutex);
> + return ret;
> + }
> +
> + /* Initialise Miphy for PCIe or SATA */
> + if (miphy_phy->type == MIPHY_TYPE_PCIE)
> + miphy365x_init_pcie_port(miphy_phy, miphy_dev);
> + else
> + ret = miphy365x_init_sata_port(miphy_phy, miphy_dev);
> +
> + mutex_unlock(&miphy_dev->miphy_mutex);
> +
> + return ret;
> +}
> +
> +static struct phy *miphy365x_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct miphy365x_dev *state = dev_get_drvdata(dev);
> + u8 port, type;
> +
> + if (args->count != 2) {
> + dev_err(dev, "Invalid number of cells in 'phy' property\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (args->args[0] & 0xFFFFFF00 || args->args[1] & 0xFFFFFF00) {
> + dev_err(dev, "Unsupported flags set in 'phy' property\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + port = args->args[0];
> + type = args->args[1];
> +
> + if (WARN_ON(port >= ARRAY_SIZE(ports)))
> + return ERR_PTR(-EINVAL);
> +
> + if (type == MIPHY_TYPE_SATA)
> + state->phys[port].base = state->phys[port].sata;
> + else if (type == MIPHY_TYPE_PCIE)
> + state->phys[port].base = state->phys[port].pcie;
Sergei made an important point about overriding the PHY mode (like here) that
might create problem for the first driver that got the PHY. This might need
both the phy-core and the phy driver to maintain state to handle this properly.
> + else {
> + WARN(1, "Invalid type specified in DT");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + state->phys[port].type = type;
> +
> + return state->phys[port].phy;
> +}
> +
> +static struct phy_ops miphy365x_ops = {
> + .init = miphy365x_init,
> + .owner = THIS_MODULE,
> +};
> +
> +static int miphy365x_get_base_addr(struct platform_device *pdev,
> + struct miphy365x_phy *phy, u8 port)
> +{
> + struct resource *res;
> + char type[6];
> +
> + sprintf(type, "sata%d", port);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> + if (!res)
> + return -ENODEV;
> +
> + phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!phy->sata)
> + return -ENOMEM;
use devm_ioremap_resource?
> +
> + sprintf(type, "pcie%d", port);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> + if (!res)
> + return -ENODEV;
> +
> + phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!phy->pcie)
> + return -ENOMEM;
here too..
> +
> + return 0;
> +}
> +
> +static int miphy365x_of_probe(struct device_node *np,
> + struct miphy365x_dev *phy_dev)
> +{
> + phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> + if (IS_ERR(phy_dev->regmap)) {
> + dev_err(phy_dev->dev, "No syscfg phandle specified\n");
> + return PTR_ERR(phy_dev->regmap);
> + }
> +
> + of_property_read_u32(np, "st,sata-gen", &phy_dev->sata_gen);
> + if (!phy_dev->sata_gen)
> + phy_dev->sata_gen = SATA_GEN1;
> +
> + phy_dev->pcie_tx_pol_inv =
> + of_property_read_bool(np, "st,pcie-tx-pol-inv");
> +
> + phy_dev->sata_tx_pol_inv =
> + of_property_read_bool(np, "st,sata-tx-pol-inv");
> +
> + return 0;
> +}
> +
> +static int miphy365x_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct miphy365x_dev *phy_dev;
> + struct device *dev = &pdev->dev;
> + struct phy_provider *provider;
> + u8 port;
> + int ret;
> +
> + if (!np) {
> + dev_err(dev, "No DT node found\n");
> + return -EINVAL;
> + }
This check is not needed as you have depends of OF.
> +
> + phy_dev = devm_kzalloc(dev, sizeof(*phy_dev), GFP_KERNEL);
> + if (!phy_dev)
> + return -ENOMEM;
> +
> + ret = miphy365x_of_probe(np, phy_dev);
> + if (ret)
> + return ret;
> +
> + phy_dev->dev = dev;
> +
> + dev_set_drvdata(dev, phy_dev);
> +
> + mutex_init(&phy_dev->miphy_mutex);
> +
> + for (port = 0; port < ARRAY_SIZE(ports); port++) {
Pls refer [1]. I feel it's better to do it that way.
[1] -> http://www.spinics.net/lists/linux-sh/msg33350.html
Cheers
Kishon
Hi,
On Monday 30 June 2014 08:11 PM, Lee Jones wrote:
> phy: miphy365x: Represent each PHY channel as a DT subnode
>
> This has the added advantages of being able to enable/disable each
> of the channels as simply as enabling/disabling the DT node.
>
> Suggested-by: Kishon Vijay Abraham I <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
>
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> index 1109f42..2c4ea6e 100644
> --- a/drivers/phy/phy-miphy365x.c
> +++ b/drivers/phy/phy-miphy365x.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/of_address.h>
> #include <linux/clk.h>
> #include <linux/phy/phy.h>
> #include <linux/delay.h>
> @@ -144,6 +145,7 @@ struct miphy365x {
> void __iomem *pcie;
> u8 type;
> u8 port;
> + bool enabled;
> };
>
> struct miphy365x_dev {
> @@ -468,7 +470,7 @@ static struct phy *miphy365x_xlate(struct device *dev,
> struct miphy365x_dev *state = dev_get_drvdata(dev);
> u8 port, type;
>
> - if (args->count != 2) {
> + if (args->args_count != 2) {
> dev_err(dev, "Invalid number of cells in 'phy' property\n");
> return ERR_PTR(-EINVAL);
> }
> @@ -484,6 +486,11 @@ static struct phy *miphy365x_xlate(struct device *dev,
> if (WARN_ON(port >= ARRAY_SIZE(ports)))
> return ERR_PTR(-EINVAL);
>
> + if (!state->phys[port].enabled) {
> + dev_warn(dev, "PHY port %d is disabled\n", port);
> + return ERR_PTR(-EINVAL);
> + }
> +
> if (type == MIPHY_TYPE_SATA)
> state->phys[port].base = state->phys[port].sata;
> else if (type == MIPHY_TYPE_PCIE)
> @@ -503,38 +510,75 @@ static struct phy_ops miphy365x_ops = {
> .owner = THIS_MODULE,
> };
>
> -static int miphy365x_get_base_addr(struct platform_device *pdev,
> - struct miphy365x_phy *phy, u8 port)
> +static int miphy365x_get_base_addr_one(struct platform_device *pdev,
> + struct miphy365x *phy,
> + struct device_node *child,
> + int index)
> {
> - struct resource *res;
> - char type[6];
> + void __iomem *base;
> + const char *name;
> + int ret;
>
> - sprintf(type, "sata%d", port);
> + base = of_iomap(child, index);
> + if (!base) {
> + dev_err(&pdev->dev, "Failed to map %s\n", child->full_name);
> + return -EINVAL;
> + }
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> - if (!res)
> - return -ENODEV;
> + ret = of_property_read_string_index(child, "reg-names", index, &name);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "%s: no reg-names property not found\n", child->name);
> + return ret;
> + }
>
> - phy->sata = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!phy->sata)
> - return -ENOMEM;
> + if (!strncmp(name, "sata", 4))
> + phy->sata = base;
> + else if (!strncmp(name, "pcie", 4))
> + phy->pcie = base;
> + else {
> + dev_err(&pdev->dev, "reg-names in %s not sata or pcie: %s",
> + child->name, name);
> + return -EINVAL;
> + }
>
> - sprintf(type, "pcie%d", port);
> + return 0;
> +}
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, type);
> - if (!res)
> - return -ENODEV;
> +static int miphy365x_get_base_addr(struct platform_device *pdev,
> + struct miphy365x *phy,
> + struct device_node *child)
> +{
> + int index;
> + int ret;
>
> - phy->pcie = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> - if (!phy->pcie)
> - return -ENOMEM;
> + /* Each port handles SATA or PCIE. */
> + for (index = 0; index < 2; index++) {
> + ret = miphy365x_get_base_addr_one(pdev, phy,
> + child, index);
> + if (ret)
> + return ret;
> + }
>
> return 0;
> }
>
> -static int miphy365x_of_probe(struct device_node *np,
> +static int miphy365x_of_probe(struct platform_device *pdev,
> struct miphy365x_dev *phy_dev)
> {
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *child;
> + int child_count = 0;
> +
> + for_each_child_of_node(np, child)
> + child_count++;
use of_get_child_count() instead.
> +
> + if (child_count != ARRAY_SIZE(ports)) {
> + dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
> + ARRAY_SIZE(ports), child_count);
> + return -EINVAL;
> + }
> +
> phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> if (IS_ERR(phy_dev->regmap)) {
> dev_err(phy_dev->dev, "No syscfg phandle specified\n");
> @@ -556,11 +600,10 @@ static int miphy365x_of_probe(struct device_node *np,
>
> static int miphy365x_probe(struct platform_device *pdev)
> {
> - struct device_node *np = pdev->dev.of_node;
> + struct device_node *child, *np = pdev->dev.of_node;
> struct miphy365x_dev *phy_dev;
> struct device *dev = &pdev->dev;
> struct phy_provider *provider;
> - u8 port;
> int ret;
>
> if (!np) {
> @@ -572,7 +615,7 @@ static int miphy365x_probe(struct platform_device *pdev)
> if (!phy_dev)
> return -ENOMEM;
>
> - ret = miphy365x_of_probe(np, phy_dev);
> + ret = miphy365x_of_probe(pdev, phy_dev);
> if (ret)
> return ret;
>
> @@ -582,8 +625,9 @@ static int miphy365x_probe(struct platform_device *pdev)
>
> mutex_init(&phy_dev->miphy_mutex);
>
> - for (port = 0; port < ARRAY_SIZE(ports); port++) {
> + for_each_child_of_node(np, child) {
> struct phy *phy;
> + static int port = 0;
>
> phy = devm_phy_create(dev, &miphy365x_ops, NULL);
> if (IS_ERR(phy)) {
> @@ -591,15 +635,17 @@ static int miphy365x_probe(struct platform_device *pdev)
> return PTR_ERR(phy);
> }
>
> - phy_dev->phys[port].phy = phy;
> - phy_dev->phys[port].port = port;
> -
> - ret = miphy365x_get_base_addr(pdev,
> - &phy_dev->phys[port], port);
> + ret = miphy365x_get_base_addr(pdev, &phy_dev->phys[port],
> + child);
> if (ret)
> return ret;
>
> + phy_dev->phys[port].phy = phy;
> + phy_dev->phys[port].port = port;
> + phy_dev->phys[port].enabled = !!of_device_is_available(child);
> +
> phy_set_drvdata(phy, &phy_dev->phys[port]);
> + port++;
> }
>
> provider = devm_of_phy_provider_register(dev, miphy365x_xlate);
I think you can merge this to your original patch.
Cheers
Kishon
> On Monday 30 June 2014 08:11 PM, Lee Jones wrote:
> > phy: miphy365x: Represent each PHY channel as a DT subnode
> >
> > This has the added advantages of being able to enable/disable each
> > of the channels as simply as enabling/disabling the DT node.
> >
> > Suggested-by: Kishon Vijay Abraham I <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> >
> > diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> > index 1109f42..2c4ea6e 100644
> > --- a/drivers/phy/phy-miphy365x.c
> > +++ b/drivers/phy/phy-miphy365x.c
[...]
> > -static int miphy365x_of_probe(struct device_node *np,
> > +static int miphy365x_of_probe(struct platform_device *pdev,
> > struct miphy365x_dev *phy_dev)
> > {
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device_node *child;
> > + int child_count = 0;
> > +
> > + for_each_child_of_node(np, child)
> > + child_count++;
>
> use of_get_child_count() instead.
Ah, nice. I'll do that.
[...]
> I think you can merge this to your original patch.
I can do that, but I thought It'd be nice to keep some history and
show the migration over to a different setup. This is particularly
important for when we back-port the changes back into the internal
development kernel.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wednesday 02 July 2014 05:04 PM, Lee Jones wrote:
>> On Monday 30 June 2014 08:11 PM, Lee Jones wrote:
>>> phy: miphy365x: Represent each PHY channel as a DT subnode
>>>
>>> This has the added advantages of being able to enable/disable each
>>> of the channels as simply as enabling/disabling the DT node.
>>>
>>> Suggested-by: Kishon Vijay Abraham I <[email protected]>
>>> Signed-off-by: Lee Jones <[email protected]>
>>>
>>> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
>>> index 1109f42..2c4ea6e 100644
>>> --- a/drivers/phy/phy-miphy365x.c
>>> +++ b/drivers/phy/phy-miphy365x.c
>
> [...]
>
>>> -static int miphy365x_of_probe(struct device_node *np,
>>> +static int miphy365x_of_probe(struct platform_device *pdev,
>>> struct miphy365x_dev *phy_dev)
>>> {
>>> + struct device_node *np = pdev->dev.of_node;
>>> + struct device_node *child;
>>> + int child_count = 0;
>>> +
>>> + for_each_child_of_node(np, child)
>>> + child_count++;
>>
>> use of_get_child_count() instead.
>
> Ah, nice. I'll do that.
>
> [...]
>
>> I think you can merge this to your original patch.
>
> I can do that, but I thought It'd be nice to keep some history and
> show the migration over to a different setup. This is particularly
> important for when we back-port the changes back into the internal
> development kernel.
cool.. i'm fine with it.
Cheers
Kishon
On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> > devices. It has 2 ports which it can use for either; both SATA, both
> > PCIe or one of each in any configuration.
> >
> > Acked-by: Kishon Vijay Abraham I <[email protected]>
I added this (a long time ago) becuase you already accepted this patch
into your tree at one point. I guess things have changed since then,
I'll remove for the next submission.
> > Acked-by: Mark Rutland <[email protected]>
> > Signed-off-by: Alexandre Torgue <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/phy/Kconfig | 10 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 641 insertions(+)
> > create mode 100644 drivers/phy/phy-miphy365x.c
[...]
> > +static void miphy365x_init_pcie_port(struct miphy365x *miphy_phy,
> > + struct miphy365x_dev *miphy_dev)
>
> I would have this return int so that it's API is similar to that of sata.
Seems a little pointless, but okay.
[...]
> > +static inline int miphy365x_hfc_not_rdy(struct miphy365x *miphy_phy,
> > + struct miphy365x_dev *miphy_dev)
> > +{
> > + int timeout = HFC_TIMEOUT;
> > + u8 mask = IDLL_RDY | PLL_RDY;
> > + u8 regval;
> > +
> > + do {
> > + regval = readb_relaxed(miphy_phy->base + STATUS_REG);
> > + usleep_range(2000, 2500);
>
> msleep(2)?
usleep_range() is kinder to the scheduler than msleep().
[...]
> > +static inline void miphy365x_set_comp(struct miphy365x *miphy_phy,
> > + struct miphy365x_dev *miphy_dev)
> > +{
> > + u8 val, mask;
> > +
> > + if (miphy_dev->sata_gen == SATA_GEN1)
> > + writeb_relaxed(COMP_2MHZ_RAT_GEN1,
> > + miphy_phy->base + COMP_CTRL2_REG);
> > + else
> > + writeb_relaxed(COMP_2MHZ_RAT,
> > + miphy_phy->base + COMP_CTRL2_REG);
>
> Btw don't you think it will be safe to use readb/writeb instead of
> readb_relaxed/writeb_relaxed here and everywhere else?
{read,write}b_relaxed() calls are more efficient than {read,write}b().
The non-relaxed versions are only required on architectures which do
not guarantee access ordering. This driver only supports ARM, which
does this by design.
[...]
> > + if (WARN_ON(port >= ARRAY_SIZE(ports)))
> > + return ERR_PTR(-EINVAL);
> > +
> > + if (type == MIPHY_TYPE_SATA)
> > + state->phys[port].base = state->phys[port].sata;
> > + else if (type == MIPHY_TYPE_PCIE)
> > + state->phys[port].base = state->phys[port].pcie;
>
> Sergei made an important point about overriding the PHY mode (like here) that
> might create problem for the first driver that got the PHY. This might need
> both the phy-core and the phy driver to maintain state to handle this properly.
Would you be kind enough to explain a little more about the problem
and what you think a valid solution might look like?
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> > This provides the shared header file which will be reference from both
> > the MiPHY365x driver and its associated Device Tree node(s).
> >
> > Cc: Kishon Vijay Abraham I <[email protected]>
> > Acked-by: Mark Rutland <[email protected]>
> > Acked-by: Alexandre Torgue <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > include/dt-bindings/phy/phy-miphy365x.h | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> > create mode 100644 include/dt-bindings/phy/phy-miphy365x.h
[...]
> > +#define MIPHY_SATA_PORT0 (MIPHY_TYPE_SATA) | (MIPHY_PORT_0)
> > +#define MIPHY_SATA_PORT1 (MIPHY_TYPE_SATA) | (MIPHY_PORT_1)
> > +#define MIPHY_PCIE_PORT0 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_0)
> > +#define MIPHY_PCIE_PORT1 (MIPHY_TYPE_PCIE) | (MIPHY_PORT_1)
>
> As Sergei pointed in your previous version it should be (X | Y).
Hmm, yes. I even have his email marked as 'important' and to fix that
up. Sorry about that, will fix.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> > devices. It has 2 ports which it can use for either; both SATA, both
> > PCIe or one of each in any configuration.
> >
> > Cc: Kishon Vijay Abraham I <[email protected]>
> > Acked-by: Mark Rutland <[email protected]>
> > Acked-by: Alexandre Torgue <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > .../devicetree/bindings/phy/phy-miphy365x.txt | 76 ++++++++++++++++++++++
> > 1 file changed, 76 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> >
> > diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> > new file mode 100644
> > index 0000000..d75f300
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
> > @@ -0,0 +1,76 @@
> > +STMicroelectronics STi MIPHY365x PHY binding
> > +============================================
> > +
> > +This binding describes a miphy device that is used to control PHY hardware
> > +for SATA and PCIe.
> > +
> > +Required properties:
> > +- compatible : Should be "st,miphy365x-phy"
> > +- #phy-cells : Should be 2 (See second example)
> > + First cell is the port number from:
> > + - MIPHY_PORT_0
> > + - MIPHY_PORT_1
>
> I'm just thinking if we can directly give phandle to the sub-node
> (channel0/channel1 or port0/port1) we won't need this information in the PHY
> specifier. This might need some modification in the phy-core but that can be done.
If we do that, we need a new property to identify the port number. I
figured using an existing cell to identify the port would be better
than to try an introduce yet another property.
> > + Second cell is device type from:
> > + - MIPHY_TYPE_SATA
> > + - MIPHY_TYPE_PCI
> > +- reg : Address and length of register sets for each device in
> > + "reg-names"
> > +- reg-names : The names of the register addresses corresponding to the
> > + registers filled in "reg", from:
> > + - sata0: For SATA port 0 registers
> > + - sata1: For SATA port 1 registers
> > + - pcie0: For PCIE port 0 registers
> > + - pcie1: For PCIE port 1 registers
>
> this information should be in the documentation of sub-nodes.
You're right, will fix.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
> On Wednesday 02 July 2014 05:04 PM, Lee Jones wrote:
> >> On Monday 30 June 2014 08:11 PM, Lee Jones wrote:
> >>> phy: miphy365x: Represent each PHY channel as a DT subnode
> >>>
> >>> This has the added advantages of being able to enable/disable each
> >>> of the channels as simply as enabling/disabling the DT node.
> >>>
> >>> Suggested-by: Kishon Vijay Abraham I <[email protected]>
> >>> Signed-off-by: Lee Jones <[email protected]>
> >>>
> >>> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> >>> index 1109f42..2c4ea6e 100644
> >>> --- a/drivers/phy/phy-miphy365x.c
> >>> +++ b/drivers/phy/phy-miphy365x.c
> >
> > [...]
> >
> >>> -static int miphy365x_of_probe(struct device_node *np,
> >>> +static int miphy365x_of_probe(struct platform_device *pdev,
> >>> struct miphy365x_dev *phy_dev)
> >>> {
> >>> + struct device_node *np = pdev->dev.of_node;
> >>> + struct device_node *child;
> >>> + int child_count = 0;
> >>> +
> >>> + for_each_child_of_node(np, child)
> >>> + child_count++;
> >>
> >> use of_get_child_count() instead.
> >
> > Ah, nice. I'll do that.
> >
> > [...]
> >
> >> I think you can merge this to your original patch.
> >
> > I can do that, but I thought It'd be nice to keep some history and
> > show the migration over to a different setup. This is particularly
> > important for when we back-port the changes back into the internal
> > development kernel.
>
> cool.. i'm fine with it.
Thanks.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
> > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> > devices. It has 2 ports which it can use for either; both SATA, both
> > PCIe or one of each in any configuration.
> >
> > Acked-by: Kishon Vijay Abraham I <[email protected]>
Removed.
> > Acked-by: Mark Rutland <[email protected]>
> > Signed-off-by: Alexandre Torgue <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/phy/Kconfig | 10 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 641 insertions(+)
> > create mode 100644 drivers/phy/phy-miphy365x.c
[...]
> > +struct miphy365x_dev {
> > + struct device *dev;
> > + struct mutex miphy_mutex;
> > + struct miphy365x phys[ARRAY_SIZE(ports)];
>
> Avoid using fixed array sizes for ports or channels. Refer [1].
Just addressing this point in this mail. Any other subsequent points
will either be fixed up or addressed in other correspondence.
I don't agree with this point. I don't believe the number of channels
should be dictated by the number of DT sub-nodes supplied. Instead,
the driver should contain knowledge about what is supported and
validate the DT data accordingly. If it's omitted we lose the ability
to conduct any kind of bounds checking, such like the following:
if (WARN_ON(port >= ARRAY_SIZE(ports)))
return ERR_PTR(-EINVAL);
And
if (child_count != ARRAY_SIZE(ports)) {
dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
ARRAY_SIZE(ports), child_count);
return -EINVAL;
}
If at a later point, we need to expand the driver to support a new
chip which supports more channels/ports then we need to expand the
bounds checking based on match data extracted from the supplied
compatible string. For instance, if a 4 port controller is being used
and only 2 channels have been supplied, or vice versa then probe()
should fail.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hi,
On Wednesday 02 July 2014 05:36 PM, Lee Jones wrote:
> On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
>> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
>>> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
>>> devices. It has 2 ports which it can use for either; both SATA, both
>>> PCIe or one of each in any configuration.
>>>
>>> Cc: Kishon Vijay Abraham I <[email protected]>
>>> Acked-by: Mark Rutland <[email protected]>
>>> Acked-by: Alexandre Torgue <[email protected]>
>>> Signed-off-by: Lee Jones <[email protected]>
>>> ---
>>> .../devicetree/bindings/phy/phy-miphy365x.txt | 76 ++++++++++++++++++++++
>>> 1 file changed, 76 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-miphy365x.txt b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> new file mode 100644
>>> index 0000000..d75f300
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/phy-miphy365x.txt
>>> @@ -0,0 +1,76 @@
>>> +STMicroelectronics STi MIPHY365x PHY binding
>>> +============================================
>>> +
>>> +This binding describes a miphy device that is used to control PHY hardware
>>> +for SATA and PCIe.
>>> +
>>> +Required properties:
>>> +- compatible : Should be "st,miphy365x-phy"
>>> +- #phy-cells : Should be 2 (See second example)
>>> + First cell is the port number from:
>>> + - MIPHY_PORT_0
>>> + - MIPHY_PORT_1
>>
>> I'm just thinking if we can directly give phandle to the sub-node
>> (channel0/channel1 or port0/port1) we won't need this information in the PHY
>> specifier. This might need some modification in the phy-core but that can be done.
>
> If we do that, we need a new property to identify the port number. I
> figured using an existing cell to identify the port would be better
> than to try an introduce yet another property.
If we can directly give phandle to the sub-node where do you think port number
would be used?
Thanks
Kishon
Hi,
On Thursday 03 July 2014 01:37 PM, Lee Jones wrote:
> On Wed, 02 Jul 2014, Kishon Vijay Abraham I wrote:
>> On Monday 30 June 2014 06:31 PM, Lee Jones wrote:
>>> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
>>> devices. It has 2 ports which it can use for either; both SATA, both
>>> PCIe or one of each in any configuration.
>>>
>>> Acked-by: Kishon Vijay Abraham I <[email protected]>
>
> Removed.
>
>>> Acked-by: Mark Rutland <[email protected]>
>>> Signed-off-by: Alexandre Torgue <[email protected]>
>>> Signed-off-by: Lee Jones <[email protected]>
>>> ---
>>> drivers/phy/Kconfig | 10 +
>>> drivers/phy/Makefile | 1 +
>>> drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 641 insertions(+)
>>> create mode 100644 drivers/phy/phy-miphy365x.c
>
> [...]
>
>>> +struct miphy365x_dev {
>>> + struct device *dev;
>>> + struct mutex miphy_mutex;
>>> + struct miphy365x phys[ARRAY_SIZE(ports)];
>>
>> Avoid using fixed array sizes for ports or channels. Refer [1].
>
> Just addressing this point in this mail. Any other subsequent points
> will either be fixed up or addressed in other correspondence.
>
> I don't agree with this point. I don't believe the number of channels
> should be dictated by the number of DT sub-nodes supplied. Instead,
But that's the way it is. The DT describes your hw and not the driver. However
the driver may not support everything that is in the hw.
> the driver should contain knowledge about what is supported and
> validate the DT data accordingly. If it's omitted we lose the ability
IMO the driver cannot validate DT data, it can just return error if there is
something the _driver_ cannot support.
> to conduct any kind of bounds checking, such like the following:
>
> if (WARN_ON(port >= ARRAY_SIZE(ports)))
> return ERR_PTR(-EINVAL);
Just as I mentioned in the other patch, 'ports' shouldn't be needed at all. If
we directly give phandle to the sub-node, it won't be needed.
> And
> if (child_count != ARRAY_SIZE(ports)) {
> dev_err(&pdev->dev, "%d ports supported, %d supplied\n",
> ARRAY_SIZE(ports), child_count);
> return -EINVAL;
> }
>
> If at a later point, we need to expand the driver to support a new
> chip which supports more channels/ports then we need to expand the
> bounds checking based on match data extracted from the supplied
> compatible string. For instance, if a 4 port controller is being used
> and only 2 channels have been supplied, or vice versa then probe()
> should fail.
I don't think error checking of this sort should be done in driver. The dt
_should_ know what is the controller that is being used.
Cheers
Kishon
Hi Lee,
You missed to mention the target name (stih416) in the title or in the
commit message.
BR
Gabriel
On 30 June 2014 15:01, Lee Jones <[email protected]> wrote:
> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> devices. It has 2 ports which it can use for either; both SATA, both
> PCIe or one of each in any configuration.
>
> Acked-by: Mark Rutland <[email protected]>
> Acked-by: Alexandre Torgue <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> arch/arm/boot/dts/stih416-b2020-revE.dts | 5 +++++
> arch/arm/boot/dts/stih416-b2020.dts | 7 +++++++
> arch/arm/boot/dts/stih416.dtsi | 21 +++++++++++++++++++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih416-b2020-revE.dts b/arch/arm/boot/dts/stih416-b2020-revE.dts
> index ba0fa2c..0e2c870 100644
> --- a/arch/arm/boot/dts/stih416-b2020-revE.dts
> +++ b/arch/arm/boot/dts/stih416-b2020-revE.dts
> @@ -31,5 +31,10 @@
> ethernet1: dwmac@fef08000 {
> snps,reset-gpio = <&PIO0 7>;
> };
> +
> + miphy365x_phy: miphy365x@fe382000 {
> + st,pcie-tx-pol-inv;
> + st,sata-gen = <3>;
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/stih416-b2020.dts b/arch/arm/boot/dts/stih416-b2020.dts
> index 4e2df66..6f1145c 100644
> --- a/arch/arm/boot/dts/stih416-b2020.dts
> +++ b/arch/arm/boot/dts/stih416-b2020.dts
> @@ -12,4 +12,11 @@
> / {
> model = "STiH416 B2020";
> compatible = "st,stih416-b2020", "st,stih416";
> +
> + soc {
> + miphy365x_phy: miphy365x@fe382000 {
> + st,pcie-tx-pol-inv;
> + st,sata-gen = <3>;
> + };
> + };
> };
> diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
> index 06473c5..6d7aee3 100644
> --- a/arch/arm/boot/dts/stih416.dtsi
> +++ b/arch/arm/boot/dts/stih416.dtsi
> @@ -9,6 +9,8 @@
> #include "stih41x.dtsi"
> #include "stih416-clock.dtsi"
> #include "stih416-pinctrl.dtsi"
> +
> +#include <dt-bindings/phy/phy-miphy365x.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/reset-controller/stih416-resets.h>
> / {
> @@ -236,5 +238,24 @@
> resets = <&powerdown STIH416_KEYSCAN_POWERDOWN>,
> <&softreset STIH416_KEYSCAN_SOFTRESET>;
> };
> +
> + miphy365x_phy: miphy365x@fe382000 {
> + compatible = "st,miphy365x-phy";
> + st,syscfg = <&syscfg_rear>;
> + #phy-cells = <2>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + phy_port0: port@fe382000 {
> + reg = <0xfe382000 0x100>, <0xfe394000 0x100>;
> + reg-names = "sata", "pcie";
> + };
> +
> + phy_port1: port@fe38a000 {
> + reg = <0xfe38a000 0x100>, <0xfe804000 0x100>;
> + reg-names = "sata", "pcie";
> + };
> + };
> };
> };
> --
> 1.8.3.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 30 June 2014 15:01, Lee Jones <[email protected]> wrote:
> The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> devices. It has 2 ports which it can use for either; both SATA, both
> PCIe or one of each in any configuration.
>
> Acked-by: Kishon Vijay Abraham I <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> Signed-off-by: Alexandre Torgue <[email protected]>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/phy/Kconfig | 10 +
> drivers/phy/Makefile | 1 +
> drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 641 insertions(+)
> create mode 100644 drivers/phy/phy-miphy365x.c
>
[...]
> diff --git a/drivers/phy/phy-miphy365x.c b/drivers/phy/phy-miphy365x.c
> new file mode 100644
> index 0000000..1109f42
> --- /dev/null
> +++ b/drivers/phy/phy-miphy365x.c
> @@ -0,0 +1,630 @@
> +/*
> + * Copyright (C) 2014 STMicroelectronics
> + *
> + * STMicroelectronics PHY driver MiPHY365 (for SoC STiH416).
> + *
> + * Author: Alexandre Torgue <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +#include <linux/phy/phy.h>
> +#include <linux/delay.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/phy/phy-miphy365x.h>
> +
> +#define HFC_TIMEOUT 50
> +
> +#define SYSCFG_2521 0x824
> +#define SYSCFG_2522 0x828
> +#define SYSCFG_PCIE_SATA_MASK BIT(1)
> +#define SYSCFG_PCIE_SATA_POS 1
> +
> +/* MiPHY365x register definitiona */
definition in Brazilian is 'definição', not 'definitiona' ...
:)
[...]
> +struct miphy365x {
> + struct phy *phy;
> + void __iomem *base;
> + void __iomem *sata;
> + void __iomem *pcie;
> + u8 type;
> + u8 port;
> +};
> +
> +struct miphy365x_dev {
> + struct device *dev;
> + struct mutex miphy_mutex;
> + struct miphy365x phys[ARRAY_SIZE(ports)];
> + bool pcie_tx_pol_inv;
> + bool sata_tx_pol_inv;
> + u32 sata_gen;
> + struct regmap *regmap;
> +};
> +
> +/*
[...]
> +static int miphy365x_of_probe(struct device_node *np,
> + struct miphy365x_dev *phy_dev)
> +{
> + phy_dev->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> + if (IS_ERR(phy_dev->regmap)) {
> + dev_err(phy_dev->dev, "No syscfg phandle specified\n");
> + return PTR_ERR(phy_dev->regmap);
> + }
> +
> + of_property_read_u32(np, "st,sata-gen", &phy_dev->sata_gen);
> + if (!phy_dev->sata_gen)
> + phy_dev->sata_gen = SATA_GEN1;
> +
> + phy_dev->pcie_tx_pol_inv =
> + of_property_read_bool(np, "st,pcie-tx-pol-inv");
> +
> + phy_dev->sata_tx_pol_inv =
> + of_property_read_bool(np, "st,sata-tx-pol-inv");
> +
"st,sata-gen", "st,pcie-tx-pol-inv", "st,sata-tx-pol-inv" should be
defined in the sub-node ?
if yes then declare pcie_tx_pol_inv, sata_tx_pol_inv and sata_gen
in struct miphy365x instead struct miphy365x_dev.
[...]
Best Regards
Gabriel
On Fri, 04 Jul 2014, Gabriel Fernandez wrote:
> On 30 June 2014 15:01, Lee Jones <[email protected]> wrote:
> > The MiPHY365x is a Generic PHY which can serve various SATA or PCIe
> > devices. It has 2 ports which it can use for either; both SATA, both
> > PCIe or one of each in any configuration.
> >
> > Acked-by: Kishon Vijay Abraham I <[email protected]>
> > Acked-by: Mark Rutland <[email protected]>
> > Signed-off-by: Alexandre Torgue <[email protected]>
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/phy/Kconfig | 10 +
> > drivers/phy/Makefile | 1 +
> > drivers/phy/phy-miphy365x.c | 630 ++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 641 insertions(+)
> > create mode 100644 drivers/phy/phy-miphy365x.c
All fixed, thanks Gabi.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog