diff mbox

[v2,net-next,1/2] ravb: Add tx and rx clock internal delays mode of APSR

Message ID 1485542120-10205-2-git-send-email-horms+renesas@verge.net.au (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Simon Horman Jan. 27, 2017, 6:35 p.m. UTC
From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>

This patch enables tx and rx clock internal delay modes (TDM and RDM).

This is to address a failure in the case of 1Gbps communication using the
by salvator-x board with the KSZ9031RNX phy. This has been reported to
occur with both the r8a7795 (H3) and r8a7796 (M3-W) SoCs.

With this change APSR internal delay modes are enabled for
"rgmii-id", "rgmii-rxid" and "rgmii-txid" phy modes as follows:

phy mode   | ASPR delay mode
-----------+----------------
rgmii-id   | TDM and RDM
rgmii-rxid | RDM
rgmii-txid | TDM

Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

---
v2 [Simon Horman]
* As suggested by Sergei Shtylyov
  - Add a comment to indicate that APSR_DM appears to be undocumented.
  - Move chip_id check outside of ravb_set_delay_mode for consistency
  - Call ravb_modify() once in ravb_set_delay_mode()
* Enhance comment before calls to ravb_set_delay_mode()
* Remove unnecessary break from end of switch statement.

v1 [Simon Horman]
- Combined patches
- Reworded changelog

v0 [Kazuya Mizuguchi]
---
 drivers/net/ethernet/renesas/ravb.h      | 10 ++++++++++
 drivers/net/ethernet/renesas/ravb_main.c | 24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

Comments

Sergei Shtylyov Jan. 27, 2017, 7:05 p.m. UTC | #1
On 01/27/2017 09:35 PM, Simon Horman wrote:

> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> This patch enables tx and rx clock internal delay modes (TDM and RDM).
>
> This is to address a failure in the case of 1Gbps communication using the
> by salvator-x board with the KSZ9031RNX phy. This has been reported to
> occur with both the r8a7795 (H3) and r8a7796 (M3-W) SoCs.
>
> With this change APSR internal delay modes are enabled for
> "rgmii-id", "rgmii-rxid" and "rgmii-txid" phy modes as follows:
>
> phy mode   | ASPR delay mode
> -----------+----------------
> rgmii-id   | TDM and RDM
> rgmii-rxid | RDM
> rgmii-txid | TDM
>
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> ---
> v2 [Simon Horman]
> * As suggested by Sergei Shtylyov
>   - Add a comment to indicate that APSR_DM appears to be undocumented.
>   - Move chip_id check outside of ravb_set_delay_mode for consistency
>   - Call ravb_modify() once in ravb_set_delay_mode()
> * Enhance comment before calls to ravb_set_delay_mode()

    Well, I meant to say that the comment should precede the body of the 
function, not be repeated at every call...

> * Remove unnecessary break from end of switch statement.

   Come on, you did remove all the *switch*. :-)

>
> v1 [Simon Horman]
> - Combined patches
> - Reworded changelog
>
> v0 [Kazuya Mizuguchi]
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 10 ++++++++++
>  drivers/net/ethernet/renesas/ravb_main.c | 24 ++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index f1109661a533..0525bd696d5d 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
[...]
> @@ -248,6 +249,15 @@ enum ESR_BIT {
>  	ESR_EIL		= 0x00001000,
>  };
>
> +/* APSR */
> +enum APSR_BIT {
> +	APSR_MEMS		= 0x00000002,
> +	APSR_CMSW		= 0x00000010,
> +	APSR_DM			= 0x00006000,	/* Undocumented? */
> +	APSR_DM_RDM		= 0x00002000,
> +	APSR_DM_TDM		= 0x00004000,

    The field values are also undocumented if the field is...

[...]

MBR, Sergei
Simon Horman Jan. 27, 2017, 7:40 p.m. UTC | #2
On Fri, Jan 27, 2017 at 10:05:58PM +0300, Sergei Shtylyov wrote:
> On 01/27/2017 09:35 PM, Simon Horman wrote:
> 
> >From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> >
> >This patch enables tx and rx clock internal delay modes (TDM and RDM).
> >
> >This is to address a failure in the case of 1Gbps communication using the
> >by salvator-x board with the KSZ9031RNX phy. This has been reported to
> >occur with both the r8a7795 (H3) and r8a7796 (M3-W) SoCs.
> >
> >With this change APSR internal delay modes are enabled for
> >"rgmii-id", "rgmii-rxid" and "rgmii-txid" phy modes as follows:
> >
> >phy mode   | ASPR delay mode
> >-----------+----------------
> >rgmii-id   | TDM and RDM
> >rgmii-rxid | RDM
> >rgmii-txid | TDM
> >
> >Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> >---
> >v2 [Simon Horman]
> >* As suggested by Sergei Shtylyov
> >  - Add a comment to indicate that APSR_DM appears to be undocumented.
> >  - Move chip_id check outside of ravb_set_delay_mode for consistency
> >  - Call ravb_modify() once in ravb_set_delay_mode()
> >* Enhance comment before calls to ravb_set_delay_mode()
> 
>    Well, I meant to say that the comment should precede the body of the
> function, not be repeated at every call...

I scratched my head trying to work out what you meant... and guessed wrong.

> >* Remove unnecessary break from end of switch statement.
> 
>   Come on, you did remove all the *switch*. :-)

Yes, after I wrote the comment above :)
diff mbox

Patch

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index f1109661a533..0525bd696d5d 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -76,6 +76,7 @@  enum ravb_reg {
 	CDAR20	= 0x0060,
 	CDAR21	= 0x0064,
 	ESR	= 0x0088,
+	APSR	= 0x008C,	/* R-Car Gen3 only */
 	RCR	= 0x0090,
 	RQC0	= 0x0094,
 	RQC1	= 0x0098,
@@ -248,6 +249,15 @@  enum ESR_BIT {
 	ESR_EIL		= 0x00001000,
 };
 
+/* APSR */
+enum APSR_BIT {
+	APSR_MEMS		= 0x00000002,
+	APSR_CMSW		= 0x00000010,
+	APSR_DM			= 0x00006000,	/* Undocumented? */
+	APSR_DM_RDM		= 0x00002000,
+	APSR_DM_TDM		= 0x00004000,
+};
+
 /* RCR */
 enum RCR_BIT {
 	RCR_EFFS	= 0x00000001,
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 89ac1e3f6175..f655c1c23765 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1904,6 +1904,22 @@  static void ravb_set_config_mode(struct net_device *ndev)
 	}
 }
 
+static void ravb_set_delay_mode(struct net_device *ndev)
+{
+	struct ravb_private *priv = netdev_priv(ndev);
+	int set = 0;
+
+	if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    priv->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID)
+		set |= APSR_DM_RDM;
+
+	if (priv->phy_interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    priv->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID)
+		set |= APSR_DM_TDM;
+
+	ravb_modify(ndev, APSR, APSR_DM, set);
+}
+
 static int ravb_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -2016,6 +2032,10 @@  static int ravb_probe(struct platform_device *pdev)
 	/* Request GTI loading */
 	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 
+	if (priv->chip_id != RCAR_GEN2)
+		/* Set tx and rx clock internal delay modes */
+		ravb_set_delay_mode(ndev);
+
 	/* Allocate descriptor base address table */
 	priv->desc_bat_size = sizeof(struct ravb_desc) * DBAT_ENTRY_NUM;
 	priv->desc_bat = dma_alloc_coherent(ndev->dev.parent, priv->desc_bat_size,
@@ -2152,6 +2172,10 @@  static int __maybe_unused ravb_resume(struct device *dev)
 	/* Request GTI loading */
 	ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
 
+	if (priv->chip_id != RCAR_GEN2)
+		/* Set tx and rx clock internal delay modes */
+		ravb_set_delay_mode(ndev);
+
 	/* Restore descriptor base address table */
 	ravb_write(ndev, priv->desc_bat_dma, DBAT);