diff mbox series

[5.10.y-cip,12/12] ravb: Add RZ/G2L MII interface support

Message ID 20221025144131.1309463-13-biju.das.jz@bp.renesas.com (mailing list archive)
State Accepted
Delegated to: Pavel Machek
Headers show
Series RZ/G2{L,LC,UL}/RZ/V2L improvements | expand

Commit Message

Biju Das Oct. 25, 2022, 2:41 p.m. UTC
commit 1089877ada8d2da5f173922f1061ccfe721ebf56 upstream.

EMAC IP found on RZ/G2L Gb ethernet supports MII interface.
This patch adds support for selecting MII interface mode.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
Link: https://lore.kernel.org/r/20220914192604.265859-1-biju.das.jz@bp.renesas.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/net/ethernet/renesas/ravb.h      | 8 ++++++++
 drivers/net/ethernet/renesas/ravb_main.c | 8 +++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

Comments

Nobuhiro Iwamatsu Oct. 26, 2022, 8:54 a.m. UTC | #1
Hi Biju,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Tuesday, October 25, 2022 11:42 PM
> To: cip-dev@lists.cip-project.org; iwamatsu nobuhiro(岩松 信洋 □SWC◯A
> CT) <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek
> <pavel@denx.de>
> Cc: Chris Paterson <chris.paterson2@renesas.com>; Biju Das
> <biju.das@bp.renesas.com>
> Subject: [PATCH 5.10.y-cip 12/12] ravb: Add RZ/G2L MII interface support
> 
> commit 1089877ada8d2da5f173922f1061ccfe721ebf56 upstream.
> 
> EMAC IP found on RZ/G2L Gb ethernet supports MII interface.
> This patch adds support for selecting MII interface mode.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Link:
> https://lore.kernel.org/r/20220914192604.265859-1-biju.das.jz@bp.renesas.c
> om
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/net/ethernet/renesas/ravb.h      | 8 ++++++++
>  drivers/net/ethernet/renesas/ravb_main.c | 8 +++++++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb.h
> b/drivers/net/ethernet/renesas/ravb.h
> index 0b5801f7e961..76ae3b5c568e 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
> @@ -189,6 +189,7 @@ enum ravb_reg {
>  	PSR	= 0x0528,
>  	PIPR	= 0x052c,
>  	CXR31	= 0x0530,	/* RZ/G2L only */
> +	CXR35	= 0x0540,	/* RZ/G2L only */
>  	MPR	= 0x0558,
>  	PFTCR	= 0x055c,
>  	PFRCR	= 0x0560,
> @@ -965,6 +966,13 @@ enum CXR31_BIT {
>  	CXR31_SEL_LINK1	= 0x00000008,
>  };
> 
> +enum CXR35_BIT {
> +	CXR35_SEL_XMII		= 0x00000003,
> +	CXR35_SEL_XMII_RGMII	= 0x00000000,
> +	CXR35_SEL_XMII_MII	= 0x00000002,
> +	CXR35_HALFCYC_CLKSW	= 0xffff0000,
> +};
> +
>  enum CSR0_BIT {
>  	CSR0_TPE	= 0x00000010,
>  	CSR0_RPE	= 0x00000020,
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> b/drivers/net/ethernet/renesas/ravb_main.c
> index ea76fd98a767..bc259105a55b 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -537,7 +537,13 @@ static void ravb_emac_init_gbeth(struct net_device
> *ndev)
>  	/* E-MAC interrupt enable register */
>  	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
> 
> -	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 | CXR31_SEL_LINK1,
> CXR31_SEL_LINK0);
> +	if (priv->phy_interface == PHY_INTERFACE_MODE_MII) {
> +		ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 |
> CXR31_SEL_LINK1, 0);
> +		ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII,
> CXR35);

"(1000 << 16)" is a magic number, so I suggest defining it or writing a comment.

> +	} else {
> +		ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 |
> CXR31_SEL_LINK1,
> +			    CXR31_SEL_LINK0);
> +	}
>  }
> 
>  static void ravb_emac_init_rcar(struct net_device *ndev)
> --
> 2.25.1

Best regards,
  Nobuhiro
Biju Das Oct. 26, 2022, 9:09 a.m. UTC | #2
Hi Nobuhiro-San,

Thanks for the feedback.

> Subject: RE: [PATCH 5.10.y-cip 12/12] ravb: Add RZ/G2L MII interface
> support
> 
> Hi Biju,
> 
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> > Sent: Tuesday, October 25, 2022 11:42 PM
> > To: cip-dev@lists.cip-project.org; iwamatsu nobuhiro(岩松 信洋 □SWC◯
> A
> > CT) <nobuhiro1.iwamatsu@toshiba.co.jp>; Pavel Machek
> <pavel@denx.de>
> > Cc: Chris Paterson <chris.paterson2@renesas.com>; Biju Das
> > <biju.das@bp.renesas.com>
> > Subject: [PATCH 5.10.y-cip 12/12] ravb: Add RZ/G2L MII interface
> > support
> >
> > commit 1089877ada8d2da5f173922f1061ccfe721ebf56 upstream.
> >
> > EMAC IP found on RZ/G2L Gb ethernet supports MII interface.
> > This patch adds support for selecting MII interface mode.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> > Link:
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      | 8 ++++++++
> >  drivers/net/ethernet/renesas/ravb_main.c | 8 +++++++-
> >  2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index 0b5801f7e961..76ae3b5c568e 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -189,6 +189,7 @@ enum ravb_reg {
> >  	PSR	= 0x0528,
> >  	PIPR	= 0x052c,
> >  	CXR31	= 0x0530,	/* RZ/G2L only */
> > +	CXR35	= 0x0540,	/* RZ/G2L only */
> >  	MPR	= 0x0558,
> >  	PFTCR	= 0x055c,
> >  	PFRCR	= 0x0560,
> > @@ -965,6 +966,13 @@ enum CXR31_BIT {
> >  	CXR31_SEL_LINK1	= 0x00000008,
> >  };
> >
> > +enum CXR35_BIT {
> > +	CXR35_SEL_XMII		= 0x00000003,
> > +	CXR35_SEL_XMII_RGMII	= 0x00000000,
> > +	CXR35_SEL_XMII_MII	= 0x00000002,
> > +	CXR35_HALFCYC_CLKSW	= 0xffff0000,
> > +};
> > +
> >  enum CSR0_BIT {
> >  	CSR0_TPE	= 0x00000010,
> >  	CSR0_RPE	= 0x00000020,
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index ea76fd98a767..bc259105a55b 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -537,7 +537,13 @@ static void ravb_emac_init_gbeth(struct
> > net_device
> > *ndev)
> >  	/* E-MAC interrupt enable register */
> >  	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
> >
> > -	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 | CXR31_SEL_LINK1,
> > CXR31_SEL_LINK0);
> > +	if (priv->phy_interface == PHY_INTERFACE_MODE_MII) {
> > +		ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 |
> > CXR31_SEL_LINK1, 0);
> > +		ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII,
> > CXR35);
> 
> "(1000 << 16)" is a magic number, so I suggest defining it or writing
> a comment.

Renesas RAVB reviewer doesn't like macro, he wants plain number here.

See [1]

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220914064730.1878211-1-biju.das.jz@bp.renesas.com/

Cheers,
Biju
Nobuhiro Iwamatsu Oct. 26, 2022, 9:17 a.m. UTC | #3
Hi Biju,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Wednesday, October 26, 2022 6:10 PM
> To: iwamatsu nobuhiro(岩松 信洋 □SWC◯ACT)
> <nobuhiro1.iwamatsu@toshiba.co.jp>; cip-dev@lists.cip-project.org;
> pavel@denx.de
> Cc: Chris Paterson <Chris.Paterson2@renesas.com>;
> biju.das@bp.renesas.com
> Subject: RE: [PATCH 5.10.y-cip 12/12] ravb: Add RZ/G2L MII interface support
> 
> Hi Nobuhiro-San,
> 
> Thanks for the feedback.
> 

<snip>

> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index 0b5801f7e961..76ae3b5c568e 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > > @@ -189,6 +189,7 @@ enum ravb_reg {
> > >  	PSR	= 0x0528,
> > >  	PIPR	= 0x052c,
> > >  	CXR31	= 0x0530,	/* RZ/G2L only */
> > > +	CXR35	= 0x0540,	/* RZ/G2L only */
> > >  	MPR	= 0x0558,
> > >  	PFTCR	= 0x055c,
> > >  	PFRCR	= 0x0560,
> > > @@ -965,6 +966,13 @@ enum CXR31_BIT {
> > >  	CXR31_SEL_LINK1	= 0x00000008,
> > >  };

<snip>

> > > CXR31_SEL_LINK1, 0);
> > > +		ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII,
> > > CXR35);
> >
> > "(1000 << 16)" is a magic number, so I suggest defining it or writing
> > a comment.
> 
> Renesas RAVB reviewer doesn't like macro, he wants plain number here.
> 
> See [1]
> 
> [1]
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220914064
> 730.1878211-1-biju.das.jz@bp.renesas.com/

Thanks for letting me know about the email you discussed.
Got it.

Best regards,
  Nobuhiro
diff mbox series

Patch

diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 0b5801f7e961..76ae3b5c568e 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -189,6 +189,7 @@  enum ravb_reg {
 	PSR	= 0x0528,
 	PIPR	= 0x052c,
 	CXR31	= 0x0530,	/* RZ/G2L only */
+	CXR35	= 0x0540,	/* RZ/G2L only */
 	MPR	= 0x0558,
 	PFTCR	= 0x055c,
 	PFRCR	= 0x0560,
@@ -965,6 +966,13 @@  enum CXR31_BIT {
 	CXR31_SEL_LINK1	= 0x00000008,
 };
 
+enum CXR35_BIT {
+	CXR35_SEL_XMII		= 0x00000003,
+	CXR35_SEL_XMII_RGMII	= 0x00000000,
+	CXR35_SEL_XMII_MII	= 0x00000002,
+	CXR35_HALFCYC_CLKSW	= 0xffff0000,
+};
+
 enum CSR0_BIT {
 	CSR0_TPE	= 0x00000010,
 	CSR0_RPE	= 0x00000020,
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index ea76fd98a767..bc259105a55b 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -537,7 +537,13 @@  static void ravb_emac_init_gbeth(struct net_device *ndev)
 	/* E-MAC interrupt enable register */
 	ravb_write(ndev, ECSIPR_ICDIP, ECSIPR);
 
-	ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 | CXR31_SEL_LINK1, CXR31_SEL_LINK0);
+	if (priv->phy_interface == PHY_INTERFACE_MODE_MII) {
+		ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 | CXR31_SEL_LINK1, 0);
+		ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII, CXR35);
+	} else {
+		ravb_modify(ndev, CXR31, CXR31_SEL_LINK0 | CXR31_SEL_LINK1,
+			    CXR31_SEL_LINK0);
+	}
 }
 
 static void ravb_emac_init_rcar(struct net_device *ndev)