MinnowBoard Support V2 (Serial and Ethernet)

Use the DMI interface rather than manually matching DMI strings.

Signed-off-by Darren Hart <[email protected]>

Cc Michael Brunner <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

---
 drivers/tty/serial/pch_uart.c | 71 +++++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 22 deletions(-)

drivers/tty/serial/pch_uart.c +49 -22

index 572d481..271cc73 100644
@@ -373,35 +373,62 @@ static const struct file_operations port_regs_ops = {
 };
 #endif	/* CONFIG_DEBUG_FS */
 
+static struct dmi_system_id __initdata pch_uart_dmi_table[] = {
+	{
+		.ident = "CM-iTC",
+		{
+			DMI_MATCH(DMI_BOARD_NAME, "CM-iTC"),
+		},
+		(void *)CMITC_UARTCLK,
+	},
+	{
+		.ident = "FRI2",
+		{
+			DMI_MATCH(DMI_BIOS_VERSION, "FRI2"),
+		},
+		(void *)FRI2_64_UARTCLK,
+	},
+	{
+		.ident = "Fish River Island II",
+		{
+			DMI_MATCH(DMI_PRODUCT_NAME, "Fish River Island II"),
+		},
+		(void *)FRI2_48_UARTCLK,
+	},
+	{
+		.ident = "COMe-mTT",
+		{
+			DMI_MATCH(DMI_BOARD_NAME, "COMe-mTT"),
+		},
+		(void *)NTC1_UARTCLK,
+	},
+	{
+		.ident = "nanoETXexpress-TT",
+		{
+			DMI_MATCH(DMI_BOARD_NAME, "nanoETXexpress-TT"),
+		},
+		(void *)NTC1_UARTCLK,
+	},
+	{
+		.ident = "MinnowBoard",
+		{
+			DMI_MATCH(DMI_BOARD_NAME, "MinnowBoard"),
+		},
+		(void *)MINNOW_UARTCLK,
+	},
+};
+
 /* Return UART clock, checking for board specific clocks. */
 static int pch_uart_get_uartclk(void)
 {
-	const char *cmp;
+	const struct dmi_system_id *d;
 
 	if (user_uartclk)
 		return user_uartclk;
 
-	cmp = dmi_get_system_info(DMI_BOARD_NAME);
-	if (cmp && strstr(cmp, "CM-iTC"))
-		return CMITC_UARTCLK;
-
-	cmp = dmi_get_system_info(DMI_BIOS_VERSION);
-	if (cmp && strnstr(cmp, "FRI2", 4))
-		return FRI2_64_UARTCLK;
-
-	cmp = dmi_get_system_info(DMI_PRODUCT_NAME);
-	if (cmp && strstr(cmp, "Fish River Island II"))
-		return FRI2_48_UARTCLK;
-
-	/* Kontron COMe-mTT10 (nanoETXexpress-TT) */
-	cmp = dmi_get_system_info(DMI_BOARD_NAME);
-	if (cmp && (strstr(cmp, "COMe-mTT") ||
-		    strstr(cmp, "nanoETXexpress-TT")))
-		return NTC1_UARTCLK;
-
-	cmp = dmi_get_system_info(DMI_BOARD_NAME);
-	if (cmp && strstr(cmp, "MinnowBoard"))
-		return MINNOW_UARTCLK;
+	d = dmi_first_match(pch_uart_dmi_table);
+	if (d)
+		return (int)d->driver_data;
 
 	return DEFAULT_UARTCLK;
 }

Quoted 1 line · Darren Hart

Use the DMI interface rather than manually matching DMI strings.

Greg,

The rest of this series is either being dropped or has moved on to netdev. This patch however remains unchanged. Would you like me to resend individually or can you pull in this one?

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)

+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)

+int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw)

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +2 -0

+		netdev_err(adapter->netdev,
+			   "Could not disable PHY hibernation.\n");

Quoted 4 lines · Darren Hart

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

trivial comments only:

Please use scripts/checkpatch.pl

[]

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c[] renamed from drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +0 -0

[]

Quoted 1 line

+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)

[]

Review

Quoted 1 line

(quoted diff) +1 -0

+	if (ret){

Missing space before brace

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 2 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0

[]

Quoted 1 line

+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)

[]

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 3 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +3 -0

+	case PHY_AR803X_ID:
+		netdev_dbg(adapter->netdev,
+			   "Configuring AR803X PHY for 2ns TX clock delay\n"); 

[]

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 3 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +3 -0

+		netdev_err(adapter->netdev,
+			   "Unknown PHY (%x), could not set TX clock delay.\n",
+			   hw->phy.id);

[]

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 2 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +2 -0

+		netdev_err(adapter->netdev,
+			   "Could not configure tx clock delay for PHY.\n");

[]

Quoted 1 line

+int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw)

[]

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 3 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +3 -0

+	case PHY_AR803X_ID:
+		netdev_dbg(adapter->netdev,
+			   "Disabling hibernation for AR803X PHY\n");

It'd be nice if no period before newline were used everywhere.

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c

Quoted 4 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +3 -0

+		netdev_err(adapter->netdev,
+			   "Unknown PHY (%x), could not disable hibernation\n",
+			   hw->phy.id);

[]

Quoted 2 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +2 -0

+		netdev_err(adapter->netdev,
+			   "Could not disable PHY hibernation.\n");

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Signed-off-by Darren Hart <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Waskiewicz <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc <[email protected]> # 3.8.x: 5829e9b mfd: lpc_sch: Accomodate partial

Cc <[email protected]> # 3.8.x: 3cbf182 gpio-sch: Allow for more than 8

Cc <[email protected]> # 3.8.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.8.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.8.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.8.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.8.x

Cc <[email protected]> # 3.10.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.10.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.10.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.10.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.10.x

Signed-off-by Darren Hart <[email protected]>

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 15 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 98 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+)

Quoted 33 lines · Darren Hart

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Signed-off-by Darren Hart <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Waskiewicz <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc <[email protected]> # 3.8.x: 5829e9b mfd: lpc_sch: Accomodate partial

Cc <[email protected]> # 3.8.x: 3cbf182 gpio-sch: Allow for more than 8

Cc <[email protected]> # 3.8.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.8.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.8.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.8.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.8.x

Cc <[email protected]> # 3.10.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.10.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.10.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.10.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.10.x

Signed-off-by Darren Hart <[email protected]>

Quoted 6 lines

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 15 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 98 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+)

This is _far_ more than just a simple "add a new device id" for a stable
kernel update. Please go read Documentation/stable_kernel_rules.txt
again for why there's no way I can take this type of thing.

You know better than this.

Quoted 41 lines · Greg KH

Quoted 39 lines · Darren Hart

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Signed-off-by Darren Hart <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Waskiewicz <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc <[email protected]> # 3.8.x: 5829e9b mfd: lpc_sch: Accomodate partial

Cc <[email protected]> # 3.8.x: 3cbf182 gpio-sch: Allow for more than 8

Cc <[email protected]> # 3.8.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.8.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.8.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.8.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.8.x

Cc <[email protected]> # 3.10.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.10.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.10.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.10.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.10.x

Signed-off-by Darren Hart <[email protected]>

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 15 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 98 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+)

Quoted 5 lines

This is _far_ more than just a simple "add a new device id" for a stable
kernel update. Please go read Documentation/stable_kernel_rules.txt
again for why there's no way I can take this type of thing.

You know better than this.

I do appreciate the documentation that is there, and I have read it (several times). The first two for 3.8 should be acceptable.

Quoted 42 lines · Darren Hart

Quoted 41 lines · Greg KH

Quoted 39 lines · Darren Hart

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Signed-off-by Darren Hart <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Waskiewicz <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

Cc <[email protected]> # 3.8.x: 5829e9b mfd: lpc_sch: Accomodate partial

Cc <[email protected]> # 3.8.x: 3cbf182 gpio-sch: Allow for more than 8

Cc <[email protected]> # 3.8.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.8.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.8.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.8.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.8.x

Cc <[email protected]> # 3.10.x: 91bbe92: PCI: Add CircuitCo vendor ID

Cc <[email protected]> # 3.10.x: bd79680: pch_gbe: remove inline keyword

Cc <[email protected]> # 3.10.x: 453ca93: pch_gbe: convert pr_* to

Cc <[email protected]> # 3.10.x: 29cc436: pch_gbe: use managed functions

Cc <[email protected]> # 3.10.x

Signed-off-by Darren Hart <[email protected]>

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 15 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 98 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+)

Quoted 8 lines

Quoted 5 lines

This is _far_ more than just a simple "add a new device id" for a stable
kernel update. Please go read Documentation/stable_kernel_rules.txt
again for why there's no way I can take this type of thing.

You know better than this.

I do appreciate the documentation that is there, and I have read it (several times). The first two for 3.8 should be acceptable.

I didn't even look at those other patches (and hint, 3.8 is dead and burried, no stable releases are happening for it that really matter.)

I'm talking about _this_ patch. Look at it. How big it is. How it's not just "add a new device id." Now please explain how _this_ patch meets the stable kernel rules as documented? Let alone the pr_* conversions, ick, don't get me started...

Would you really send something like this to Linus after -rc4? If not, then it's not a stable patch. See the 3.10.1-rc1 thread on lkml for the details as to why I now need to push back and be harsher for this.

Just have users use 3.11, it's not like you have shipping hardware yet,

and again, who cares about 3.8. Heck, who cares about 3.9 or even 3.10 for brand-new hardware. Just use 3.11 or 3.12 and don't worry about backport hell.

This isn't going to land in Linus's tree until 3.12 anyway, so what's the rush?

Quoted 18 lines · Greg KH

Quoted 8 lines · Darren Hart

Quoted 7 lines · Greg KH

Quoted 6 lines

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 15 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 48 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 98 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+)

Quoted 8 lines

Quoted 5 lines

This is _far_ more than just a simple "add a new device id" for a stable
kernel update. Please go read Documentation/stable_kernel_rules.txt
again for why there's no way I can take this type of thing.

You know better than this.

I do appreciate the documentation that is there, and I have read it (several times). The first two for 3.8 should be acceptable.

Quoted 7 lines

I didn't even look at those other patches (and hint, 3.8 is dead and burried, no stable releases are happening for it that really matter.)

I'm talking about _this_ patch. Look at it. How big it is. How it's not just "add a new device id." Now please explain how _this_ patch meets the stable kernel rules as documented? Let alone the pr_* conversions, ick, don't get me started...

I was looking at it as a quirk:

" - New device IDs and quirks are also accepted."

I even considered implementation as a pci quirk. I didn't because the PHY work needed to happen too late during probe. The frustrating thing is there is probably 15 lines of code that are needed to get it to work, all the rest is infrastructure to make it generic.

I get the size argument. It's too big. The docs say 100 lines with context, I've seen much larger go in... I just wasn't sure where the line is. It seems things are getting more strict here, not less. OK. Lesson learned.

Quoted 1 line

Just have users use 3.11, it's not like you have shipping hardware yet,

We're talking days here, but yes.

Quoted 4 lines

and again, who cares about 3.8. Heck, who cares about 3.9 or even 3.10 for brand-new hardware. Just use 3.11 or 3.12 and don't worry about backport hell.

Quoted 2 lines

This isn't going to land in Linus's tree until 3.12 anyway, so what's the rush?

My reasoning is that the BSP for this is based on 3.8. I would like to bring 3.8 in sync with master for support of this board so I can update the release BSP to use the same sources. People can use my code from the linux-yocto_3.8 standard/minnow branch, but it would be preferable if that code was also destined for upstream.

If you still feel this doesn't qualify as a "quirk", I'll drop the stable push. If you can see that argument, I can look at breaking it up into smaller chunks, or possibly reducing some of the generic bits and doing the bare minimum to get it working and follow-up in master with more generic bits.

On Sat, Jul 13, 2013 at 09:08:01AM -0700, Darren Hart wrote:

Quoted 8 lines

I was looking at it as a quirk:

" - New device IDs and quirks are also accepted."

I even considered implementation as a pci quirk. I didn't because the PHY work needed to happen too late during probe. The frustrating thing is there is probably 15 lines of code that are needed to get it to work, all the rest is infrastructure to make it generic.

163 lines of code is not a "quirk" I can accept. When I wrote, "new device ids or quirks can be accpted for stable", I meant things like commit 9e9dd0e889c76c786e8f2e164c825c3c06dea30c. That's acceptable. Not thing huge thing.

Quoted 4 lines

I get the size argument. It's too big. The docs say 100 lines with context, I've seen much larger go in... I just wasn't sure where the line is. It seems things are getting more strict here, not less. OK. Lesson learned.

You have seen larger "quirks" than this go into the stable tree? Examples for where I've been sleeping on the job would be good.

Quoted 8 lines

Quoted 2 lines

This isn't going to land in Linus's tree until 3.12 anyway, so what's the rush?

My reasoning is that the BSP for this is based on 3.8. I would like to bring 3.8 in sync with master for support of this board so I can update the release BSP to use the same sources. People can use my code from the linux-yocto_3.8 standard/minnow branch, but it would be preferable if that code was also destined for upstream.

That's your choice to pick 3.8, not upstream's (and frankly, not something that I would have picked, but that's another topic...)

You are adding functionality for new devices that take much more than a simple "add an id to a table", so no, it's not ok for stable releases.

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h +0 -0

@@ -582,6 +582,19 @@ struct pch_gbe_hw_stats {
 };
 
 /**

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +10 -0

@@ -2720,9 +2730,47 @@ err_free_netdev:
 	return ret;
 }
 
+/* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to
+ * ensure it is awake for probe and init. Request the line and reset the PHY.
+ */
+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int flags = GPIOF_DIR_OUT | GPIOF_INIT_HIGH | GPIOF_EXPORT;
+	int gpio = MINNOW_PHY_RESET_GPIO;
+
+	ret = gpio_request_one(gpio, flags, "minnow_phy_reset");

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +15 -0

+
+static struct pch_gbe_privdata pch_gbe_minnow_privdata = {
+	.phy_tx_clk_delay = true,
+	.phy_disable_hibernate = true,
+	.platform_init = pch_gbe_minnow_platform_init,
+};
+
 static DEFINE_PCI_DEVICE_TABLE(pch_gbe_pcidev_id) = {
 	{.vendor = PCI_VENDOR_ID_INTEL,
 	 .device = PCI_DEVICE_ID_INTEL_IOH1_GBE,
+	 .subvendor = PCI_VENDOR_ID_CIRCUITCO,
+	 .subdevice = PCI_SUBSYSTEM_ID_CIRCUITCO_MINNOWBOARD,
+	 .class = (PCI_CLASS_NETWORK_ETHERNET << 8),
+	 .class_mask = (0xFFFF00),
+	 .driver_data = (unsigned long) &pch_gbe_minnow_privdata
+	 },
+	{.vendor = PCI_VENDOR_ID_INTEL,
+	 .device = PCI_DEVICE_ID_INTEL_IOH1_GBE,
 	 .subvendor = PCI_ANY_ID,
 	 .subdevice = PCI_ANY_ID,
 	 .class = (PCI_CLASS_NETWORK_ETHERNET << 8),

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +12 -0

@@ -277,4 +286,93 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	pch_gbe_phy_read_reg_miic(hw, PHY_PHYSP_CONTROL, &mii_reg);
 	mii_reg |= PHYSP_CTRL_ASSERT_CRS_TX;
 	pch_gbe_phy_write_reg_miic(hw, PHY_PHYSP_CONTROL, mii_reg);
+
+	/* Setup a TX clock delay on certain platforms */
+	if (adapter->pdata->phy_tx_clk_delay)
+		pch_gbe_phy_tx_clk_delay(hw);
+}
+
+/**
+ * pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
+ * @hw:	            Pointer to the HW structure
+ * Returns
+ *	0:		Successful.
+ *	-EINVAL:	Invalid argument.

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +31 -0

+
+	switch (hw->phy.id) {
+	case PHY_AR803X_ID:
+		netdev_dbg(adapter->netdev,
+			   "Configuring AR803X PHY for 2ns TX clock delay\n"); 
+		pch_gbe_phy_read_reg_miic(hw, PHY_AR8031_DBG_OFF, &mii_reg);
+		ret = pch_gbe_phy_write_reg_miic(hw, PHY_AR8031_DBG_OFF,
+						 PHY_AR8031_SERDES);
+		if (ret)
+			break;
+
+		pch_gbe_phy_read_reg_miic(hw, PHY_AR8031_DBG_DAT, &mii_reg);
+		mii_reg |= PHY_AR8031_SERDES_TX_CLK_DLY;
+		ret = pch_gbe_phy_write_reg_miic(hw, PHY_AR8031_DBG_DAT,
+						 mii_reg);
+		break;
+	default:
+		netdev_err(adapter->netdev,
+			   "Unknown PHY (%x), could not set TX clock delay.\n",
+			   hw->phy.id);
+		return -EINVAL;
+	}
+
+	if (ret)
+		netdev_err(adapter->netdev,
+			   "Could not configure tx clock delay for PHY.\n");
+	return ret;
+}
+
+/**
+ * pch_gbe_phy_disable_hibernate - Disable the PHY low power state

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +25 -0

+	switch (hw->phy.id) {
+	case PHY_AR803X_ID:
+		netdev_dbg(adapter->netdev,
+			   "Disabling hibernation for AR803X PHY\n");
+		ret = pch_gbe_phy_write_reg_miic(hw, PHY_AR8031_DBG_OFF,
+						 PHY_AR8031_HIBERNATE);
+		if (ret)
+			break;
+
+		pch_gbe_phy_read_reg_miic(hw, PHY_AR8031_DBG_DAT, &mii_reg);
+		mii_reg &= ~PHY_AR8031_PS_HIB_EN;
+		ret = pch_gbe_phy_write_reg_miic(hw, PHY_AR8031_DBG_DAT,
+						 mii_reg);
+		break;
+	default:
+		netdev_err(adapter->netdev,
+			   "Unknown PHY (%x), could not disable hibernation\n",
+			   hw->phy.id);
+		return -EINVAL;
+	}
+
+	if (ret)
+		netdev_err(adapter->netdev,
+			   "Could not disable PHY hibernation.\n");
+	return ret;
 }

On Fri, 2013-07-12 at 17:58 -0700, Darren Hart wrote:

Quoted 15 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +15 -0

+/* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to
+ * ensure it is awake for probe and init. Request the line and reset the PHY.
+ */
+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int flags = GPIOF_DIR_OUT | GPIOF_INIT_HIGH | GPIOF_EXPORT;
+	int gpio = MINNOW_PHY_RESET_GPIO;
+
+	ret = gpio_request_one(gpio, flags, "minnow_phy_reset");
+	if (ret){
+		dev_err(&pdev->dev,
+			"ERR: Can't request PHY reset GPIO line '%d'\n", gpio);
+		return ret;
+	}

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c err_free_netdev: · line 2752

Quoted 5 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +5 -0

+
+	gpio_set_value(gpio, 0);
+	usleep_range(1250, 1500);
+	gpio_set_value(gpio, 1);
+	usleep_range(1250, 1500);

First of all, who is going to release gpio? Perhaps devm_gpio_request_one?

Quoted 4 lines

+ */

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +3 -0

+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)
+{
+	/* The RGMII interface requires a ~2ns TX clock delay. This is typically

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw) · line 310

Quoted 6 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +6 -0

+	 * done in layout with a longer trace or via PHY strapping, but can also
+	 * be done via PHY configuration registers.
+	 */
+	struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
+	u16 mii_reg;
+	int ret = 0;

No need to assign. Are you trying to shut (sparse?) warning?

Quoted 9 lines · Darren Hart

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

Quoted 4 lines

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Few comments below.

Quoted 6 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h +0 -0

@@ -582,6 +582,19 @@ struct pch_gbe_hw_stats {
 };
 

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h struct pch_gbe_hw_stats { · line 594

Quoted 11 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h +10 -0

 /**
+ * struct pch_gbe_privdata - PCI Device ID driver data
+ * @phy_tx_clk_delay:		Bool, configure the PHY TX delay in software
+ * @phy_disable_hibernate:	Bool, disable PHY hibernation
+ * @platform_init:		Platform initialization callback, called from
+ *				probe, prio to PHY initialization.
+ */
+struct pch_gbe_privdata {
+	bool phy_tx_clk_delay;
+	bool phy_disable_hibernate;
+	int(*platform_init)(struct pci_dev *pdev);

I don't like the platform_init() approach here.
I think here should be plain GPIO line number.

I'll explain below why.

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h struct pch_gbe_hw_stats { · line 620

Quoted 5 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h +1 -0

@@ -604,6 +617,7 @@ struct pch_gbe_hw_stats {
  * @rx_buffer_len:	Receive buffer length
  * @tx_queue_len:	Transmit queue length
  * @have_msi:		PCI MSI mode flag
+ * @pch_gbe_privdata	PCI Device ID driver_data

Missed colon.

Quoted 2 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +0 -0

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c static int pch_gbe_probe(struct pci_dev *pdev, · line 2643

Quoted 7 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +3 -0

@@ -2635,6 +2638,9 @@ static int pch_gbe_probe(struct pci_dev *pdev,
 	adapter->pdev = pdev;
 	adapter->hw.back = adapter;
 	adapter->hw.reg = pcim_iomap_table(pdev)[PCH_GBE_PCI_BAR];
+	adapter->pdata = (struct pch_gbe_privdata *)pci_id->driver_data;
+	if (adapter->pdata && adapter->pdata->platform_init)
+		adapter->pdata->platform_init(pdev);

Here perhaps you check pdata and GPIO line number (let's say != -1) and call GPIO request helper.

Quoted 19 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +15 -0

@@ -2720,9 +2730,47 @@ err_free_netdev:
 	return ret;
 }
 
+/* The AR803X PHY on the MinnowBoard requires a physical pin to be toggled to
+ * ensure it is awake for probe and init. Request the line and reset the PHY.
+ */
+static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
+{
+	int ret = 0;
+	int flags = GPIOF_DIR_OUT | GPIOF_INIT_HIGH | GPIOF_EXPORT;
+	int gpio = MINNOW_PHY_RESET_GPIO;
+
+	ret = gpio_request_one(gpio, flags, "minnow_phy_reset");
+	if (ret){
+		dev_err(&pdev->dev,
+			"ERR: Can't request PHY reset GPIO line '%d'\n", gpio);
+		return ret;
+	}

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c err_free_netdev: · line 2752

Quoted 5 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +5 -0

+
+	gpio_set_value(gpio, 0);
+	usleep_range(1250, 1500);
+	gpio_set_value(gpio, 1);
+	usleep_range(1250, 1500);

First of all, who is going to release gpio? Perhaps devm_gpio_request_one?

Next is the name of the function, since you are resetting PHY, what if you call it like pch_gbe_reset_by_gpio ?

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c err_free_netdev: · line 2747

Quoted 4 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +3 -0

+
+	return ret;
+}

And most important one is the ACPI case. As far as I understand Minnow board supports / will support ACPI5 variant of device enumeration. In such case the GPIO line will come in the ACPI resources. Moreover, you will have no struct pci_dev. I highly recommend to rewrite this as a generic helper, which takes GPIO line number as an input parameter and does the job.

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c err_free_netdev: · line 2764

Quoted 9 lines

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +9 -0

+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)
+{
+	/* The RGMII interface requires a ~2ns TX clock delay. This is typically
+	 * done in layout with a longer trace or via PHY strapping, but can also
+	 * be done via PHY configuration registers.
+	 */
+	struct pch_gbe_adapter *adapter = pch_gbe_hw_to_adapter(hw);
+	u16 mii_reg;
+	int ret = 0;

No need to assign. Are you trying to shut (sparse?) warning?

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.

Signed-off-by Darren Hart <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Peter Waskiewicz <[email protected]>

Cc Andy Shevchenko <[email protected]>

Cc [email protected]

The MinnowBoard uses an AR803x PHY with the PCH GBE which requires special handling. Use the MinnowBoard PCI Subsystem ID to detect this and add a pci_device_id.driver_data structure and functions to handle platform setup.

The AR803x does not implement the RGMII 2ns TX clock delay in the trace routing nor via strapping. Add a detection method for the board and the PHY and enable the TX clock delay via the registers.

This PHY will hibernate without link for 10 seconds. Ensure the PHY is awake for probe and then disable hibernation. A future improvement would be to convert pch_gbe to using PHYLIB and making sure we can wake the PHY at the necessary times rather than permanently disabling it.