Skip to contentSkip to conversation

pch_gbe: Add MinnowBoard support

PATCH Superseded Darren Hart 3 participants 12 messages Jul 22, 2013 – Jul 25, 2013

Subsystems (3)
Networking Network drivers Checkpatch

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Darren Hart

author
#0 lore raw

v2 cover pch_gbe: Add MinnowBoard support

Add support for the MinnowBoard in the pch_gbe driver. This was originally sent to LKML as part of the MinnowBoard support series. That is now partially merged and this version of the patch has been isolated from those changes and is now completely self-contained.

Thanks to everyone on Cc for their time in reviewing, the patch series is better for it.

Since the previous version:
Whitespace cleanup
Remove trailing period in messages Comment formatting (added a colon)
Use devm_gpio_request_one
Fix missing adapter->pdata NULL check in PHY Use netdev style comment blocks
Use kernel_ulong_t for driver_data cast Use unsigned for gpio and unsigned long for flags

The following changes since commit 0887a576a17965732270b2f8d37821fc02ef2feb:

net/velocity: add poll controller function for velocity nic (2013-07-19 17:34:41 -0700)

are available in the git repository at:

git://git.infradead.org/users/dvhart/linux-2.6.git minnow/net-next ssh://git.infradead.org/srv/git/users/dvhart/linux-2.6.git minnow/net-next

and browsable here:

http://git.infradead.org/users/dvhart/linux-2.6.git/shortlog/refs/heads/minnow/net-next

Darren Hart (2):
pch_gbe: Use PCH_GBE_PHY_REGS_LEN instead of 32
pch_gbe: Add MinnowBoard support

 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 14 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 52 +++++++++++-
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 96 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 163 insertions(+), 1 deletion(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v2 1/2 pch_gbe: Use PCH_GBE_PHY_REGS_LEN instead of 32

Avoid using magic numbers when we have perfectly good defines just lying around.

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]
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +1 -1
index ab1039a..749ddd9 100644
@@ -682,7 +682,7 @@ static int pch_gbe_init_phy(struct pch_gbe_adapter *adapter)
 	}
 	adapter->hw.phy.addr = adapter->mii.phy_id;
 	netdev_dbg(netdev, "phy_addr = %d\n", adapter->mii.phy_id);
-	if (addr == 32)
+	if (addr == PCH_GBE_PHY_REGS_LEN)
 		return -EAGAIN;
 	/* Selected the phy and isolate the rest */
 	for (addr = 0; addr < PCH_GBE_PHY_REGS_LEN; addr++) {
Signature · 1 line
1.8.3.1
author
#2 lore raw

PATCH v2 2/2 pch_gbe: Add MinnowBoard support

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 Joe Perches <[email protected]>
Cc [email protected]
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h    | 14 ++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c   | 50 +++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c    | 96 ++++++++++++++++++++++
 .../net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h    |  2 +
 4 files changed, 162 insertions(+)
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe.h +14 -0
index 7779036..c6a7269 100644
@@ -581,6 +581,18 @@ struct pch_gbe_hw_stats {
 	u32 intr_tcpip_err_count;
 };
 
+/* 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, prior to PHY initialization.
+ */
+struct pch_gbe_privdata {
+	bool phy_tx_clk_delay;
+	bool phy_disable_hibernate;
+	int(*platform_init)(struct pci_dev *pdev);
+};
+
 /**
  * struct pch_gbe_adapter - board specific private data structure
  * @stats_lock:	Spinlock structure for status

@@ -604,6 +616,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
  */
 
 struct pch_gbe_adapter {

@@ -631,6 +644,7 @@ struct pch_gbe_adapter {
 	int hwts_tx_en;
 	int hwts_rx_en;
 	struct pci_dev *ptp_pdev;
+	struct pch_gbe_privdata *pdata;
 };
 
 #define pch_gbe_hw_to_adapter(hw)	container_of(hw, struct pch_gbe_adapter, hw)
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c +50 -0

83 lines not on this page · raw message

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

106 lines not on this page · raw message

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h +2 -0
index 03264dc..e3e4bc9 100644
@@ -33,5 +33,7 @@ void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
 void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
 void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw);
+int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);
 
 #endif /* _PCH_GBE_PHY_H_ */
Signature · 1 line
1.8.3.1
maintainer
#3 lore raw

From: Darren Hart <[email protected]>
Date: Mon, 22 Jul 2013 14:41:59 -0700

Quoted 4 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0
index da07907..be513a9 100644

...

Quoted 17 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +13 -0
@@ -277,4 +286,91 @@ 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 && 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.
+ */
+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)

...

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h void pch_gbe_phy_power_up(struct pch_gbe_hw *hw); · line 37

Quoted 10 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h +2 -0
index 03264dc..e3e4bc9 100644
@@ -33,5 +33,7 @@ void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
 void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
 void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw);
+int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);

pch_gbe_phy_tx_clk_delay() is only invoked from pch_gbe_phy.c, where it is also implemented.

Therefore it should be marked static and the declaration removed from pch_gbe_phy.h

Thanks.

author
#4 lore raw
Quoted 41 lines · David Miller

From: Darren Hart <[email protected]>
Date: Mon, 22 Jul 2013 14:41:59 -0700

Quoted 4 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0
index da07907..be513a9 100644

...

Quoted 17 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +13 -0
@@ -277,4 +286,91 @@ 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 && 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.
+ */
+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw)

...

drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h void pch_gbe_phy_power_up(struct pch_gbe_hw *hw); · line 37

Quoted 10 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.h +2 -0
index 03264dc..e3e4bc9 100644
@@ -33,5 +33,7 @@ void pch_gbe_phy_power_up(struct pch_gbe_hw *hw);
 void pch_gbe_phy_power_down(struct pch_gbe_hw *hw);
 void pch_gbe_phy_set_rgmii(struct pch_gbe_hw *hw);
 void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw);
+int pch_gbe_phy_tx_clk_delay(struct pch_gbe_hw *hw);
+int pch_gbe_phy_disable_hibernate(struct pch_gbe_hw *hw);

pch_gbe_phy_tx_clk_delay() is only invoked from pch_gbe_phy.c, where it is also implemented.

Quoted 2 lines

Therefore it should be marked static and the declaration removed from pch_gbe_phy.h

Ah, thanks. Leftover from a more complex driver_data with pointers to these functions. I'll fix and resubmit this patch alone as V3. Otherwise, no objections?

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
maintainer
#6 lore raw
In reply to Darren Hart
Quoted 3 lines · Darren Hart

Ah, thanks. Leftover from a more complex driver_data with pointers to these functions. I'll fix and resubmit this patch alone as V3. Otherwise, no objections?

Only small nit is not using kernel-doc for this bit below:

Quoted 8 lines · Darren Hart
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0

[]
+/* 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.
+ */

author
#7 lore raw
Quoted 6 lines · Joe Perches
Quoted 3 lines · Darren Hart

Ah, thanks. Leftover from a more complex driver_data with pointers to these functions. I'll fix and resubmit this patch alone as V3. Otherwise, no objections?

Only small nit is not using kernel-doc for this bit below:

Hi Joe,

Thank you for the review.

I was following the existing style of the file (with the exception of the leading two ** as checkpatch complains. I thought erring on the side of consistency was the best course of action here. What would you suggest is the preferred approach here?

Quoted 13 lines · Darren Hart
Quoted 8 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0

[]
+/* 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.
+ */

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
maintainer
#8 lore raw
Quoted 7 lines · Darren Hart
Quoted 6 lines · Joe Perches
Quoted 3 lines · Darren Hart

Ah, thanks. Leftover from a more complex driver_data with pointers to these functions. I'll fix and resubmit this patch alone as V3. Otherwise, no objections?

Only small nit is not using kernel-doc for this bit below:

[]

Quoted 4 lines

I was following the existing style of the file (with the exception of the leading two ** as checkpatch complains. I thought erring on the side of consistency was the best course of action here. What would you suggest is the preferred approach here?

Hi Darren.

I hadn't actually looked at the rest of the unpatched file. If what you did is the same style, it's fine.

It just looked like you were trying to use kernel-doc formatting style but not using it quite correctly.

Quoted 9 lines
Quoted 9 lines · Darren Hart
Quoted 8 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0

[]
+/* 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.
+ */

This bit in what I believe kernel-doc style is would look like:

/**
* pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
* @hw: Pointer to the HW structure
*
* Return: O: Successful
* -EINVAL: Invalid argument
*/

author
#9 lore raw
Quoted 35 lines · Joe Perches
Quoted 7 lines · Darren Hart
Quoted 6 lines · Joe Perches
Quoted 3 lines · Darren Hart

Ah, thanks. Leftover from a more complex driver_data with pointers to these functions. I'll fix and resubmit this patch alone as V3. Otherwise, no objections?

Only small nit is not using kernel-doc for this bit below:

[]

Quoted 4 lines

I was following the existing style of the file (with the exception of the leading two ** as checkpatch complains. I thought erring on the side of consistency was the best course of action here. What would you suggest is the preferred approach here?

Hi Darren.

I hadn't actually looked at the rest of the unpatched file. If what you did is the same style, it's fine.

It just looked like you were trying to use kernel-doc formatting style but not using it quite correctly.

Quoted 9 lines
Quoted 9 lines · Darren Hart
Quoted 8 lines
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c +0 -0

[]
+/* 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.
+ */

This bit in what I believe kernel-doc style is would look like:

Quoted 7 lines

/**
* pch_gbe_phy_tx_clk_delay - Setup TX clock delay via the PHY
* @hw: Pointer to the HW structure
*
* Return: O: Successful
* -EINVAL: Invalid argument
*/

Right, but /**\n is discouraged? outlawed? on netdev. I'm not sure what the kernel-doc policy would be for netdev... I think a follow-up patch is in order for this driver that continues to fix a variety of formatting, coding style, and api usage violations in addition to those Andy already provided.

I'll resubmit with the fix David asked for to get this functionality in and consider a clean-up as a follow-on.

Thanks again!

Signature · 3 lines
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
maintainer
#10 lore raw

PATCH 1/1 checkpatch: Fix networking kernel-doc block comment defect

checkpatch can generate a false positive when inserting a new kernel-doc block and function above an existing kernel-doc block.

Fix it by checking that the context line is also a newly inserted line.

Reported-by Darren Hart <[email protected]>
Signed-off-by Joe Perches <[email protected]>
---
 scripts/checkpatch.pl | 1 +
 1 file changed, 1 insertion(+)
scripts/checkpatch.pl +1 -0
index 23126d4..c18e1be 100755
@@ -2074,6 +2074,7 @@ sub process {
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&
 		    $prevrawline =~ /^\+[ \t]*\/\*/ &&		#starting /*
 		    $prevrawline !~ /\*\/[ \t]*$/ &&		#no trailing */
+		    $rawline =~ /^\+/ &&			#line is new
 		    $rawline !~ /^\+[ \t]*\*/) {		#no leading *
 			WARN("NETWORKING_BLOCK_COMMENT_STYLE",
 			     "networking block comments start with * on subsequent lines\n" . $hereprev);
Signature · 1 line
1.8.1.2.459.gbcd45b4.dirty
maintainer
#11 lore raw
In reply to Darren Hart
Quoted 2 lines · Darren Hart

I was following the existing style of the file (with the exception of the leading two ** as checkpatch complains.

I just tested and duplicated that.

It's a checkpatch defect.
I'll fix it. Ignore it for now.

cheers, Joe