* [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
@ 2022-09-14 6:47 Biju Das
2022-09-14 17:08 ` Sergey Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Biju Das @ 2022-09-14 6:47 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Biju Das, Sergey Shtylyov, Lad Prabhakar, netdev,
linux-renesas-soc, Geert Uytterhoeven, Chris Paterson, Biju Das
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>
---
v2->v3:
* Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
v1->v2:
* Fixed spaces->Tab around CXR35 description.
---
drivers/net/ethernet/renesas/ravb.h | 6 ++++++
drivers/net/ethernet/renesas/ravb_main.c | 9 ++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index b980bce763d3..058aceac8c92 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,11 @@ enum CXR31_BIT {
CXR31_SEL_LINK1 = 0x00000008,
};
+enum CXR35_BIT {
+ CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi */
+ CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used */
+};
+
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 b357ac4c56c5..9a0d06dd5eb6 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -540,7 +540,14 @@ 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, CXR35_HALFCYC_CLKSW1000 | 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)
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
2022-09-14 6:47 [PATCH net-next v3] ravb: Add RZ/G2L MII interface support Biju Das
@ 2022-09-14 17:08 ` Sergey Shtylyov
2022-09-14 17:20 ` Biju Das
0 siblings, 1 reply; 6+ messages in thread
From: Sergey Shtylyov @ 2022-09-14 17:08 UTC (permalink / raw)
To: Biju Das, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Lad Prabhakar, netdev, linux-renesas-soc, Geert Uytterhoeven,
Chris Paterson, Biju Das
On 9/14/22 9:47 AM, Biju Das wrote:
> 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>
> ---
> v2->v3:
> * Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
I definitely didn't mean it done this way...
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
> index b980bce763d3..058aceac8c92 100644
> --- a/drivers/net/ethernet/renesas/ravb.h
> +++ b/drivers/net/ethernet/renesas/ravb.h
[...]
> @@ -965,6 +966,11 @@ enum CXR31_BIT {
> CXR31_SEL_LINK1 = 0x00000008,
> };
>
> +enum CXR35_BIT {
> + CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi */
No, please just declare:
CXR35_HALFCYC_CLKSW = 0xffff0000,
> + CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used */
All the other register *enum*s are declared from LSB to MSB. The comment is pretty
self-obvious here, please remove it. And declare the whole field too:
CXR35_SEL_XMII = 0x00000003,
CXR35_SEL_XMII_RGMII = 0x00000000,
CXR35_SEL_XMII_MII = 0x00000002,
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index b357ac4c56c5..9a0d06dd5eb6 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -540,7 +540,14 @@ 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, CXR35_HALFCYC_CLKSW1000 | CXR35_SEL_XMII_MII,
> + CXR35);
ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII, CXR35);
[...]
MBR, Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
2022-09-14 17:08 ` Sergey Shtylyov
@ 2022-09-14 17:20 ` Biju Das
2022-09-14 17:29 ` Biju Das
2022-09-14 17:30 ` Sergey Shtylyov
0 siblings, 2 replies; 6+ messages in thread
From: Biju Das @ 2022-09-14 17:20 UTC (permalink / raw)
To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Prabhakar Mahadev Lad, netdev, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das
Hi Sergey,
Thanks for the feedback.
> Subject: Re: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
>
> On 9/14/22 9:47 AM, Biju Das wrote:
>
> > 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>
> > ---
> > v2->v3:
> > * Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
>
> I definitely didn't mean it done this way...
>
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index b980bce763d3..058aceac8c92 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> [...]
> > @@ -965,6 +966,11 @@ enum CXR31_BIT {
> > CXR31_SEL_LINK1 = 0x00000008,
> > };
> >
> > +enum CXR35_BIT {
> > + CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi
> */
>
> No, please just declare:
>
> CXR35_HALFCYC_CLKSW = 0xffff0000,
Q1) Why do you think we should use this value for setting MII?
As per hardware manual the value you suggested is wrong for MII settings.
See page 2157
[A] The case which CXR35 SEL_XMII is used for the selection of RGMII/MII in APB Clock 100 MHz.
(1) To use RGMII interface, Set ‘H’03E8 0000’ to this register.
(2) To use MII interface, Set ‘H’03E8 0002’ to this register.
>
> > + CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used
> */
>
> All the other register *enum*s are declared from LSB to MSB. The
> comment is pretty self-obvious here, please remove it. And declare the
> whole field too:
>
> CXR35_SEL_XMII = 0x00000003,
Values 1 and 3 are reserved so we cannot use 3.
I think the current patch holds good as per the hardware manual
for selecting MII interface. Please recheck and correct me
if it is wrong.
Cheers,
Biju
> CXR35_SEL_XMII_RGMII = 0x00000000,
> CXR35_SEL_XMII_MII = 0x00000002,
>
> [...]
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index b357ac4c56c5..9a0d06dd5eb6 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -540,7 +540,14 @@ 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, CXR35_HALFCYC_CLKSW1000 | CXR35_SEL_XMII_MII,
> > + CXR35);
>
> ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII, CXR35);
>
> [...]
>
> MBR, Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
2022-09-14 17:20 ` Biju Das
@ 2022-09-14 17:29 ` Biju Das
2022-09-14 17:30 ` Sergey Shtylyov
1 sibling, 0 replies; 6+ messages in thread
From: Biju Das @ 2022-09-14 17:29 UTC (permalink / raw)
To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Prabhakar Mahadev Lad, netdev, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das
Hi Sergey,
> Subject: RE: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
>
> Hi Sergey,
>
> Thanks for the feedback.
>
> > Subject: Re: [PATCH net-next v3] ravb: Add RZ/G2L MII interface
> > support
> >
> > On 9/14/22 9:47 AM, Biju Das wrote:
> >
> > > 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>
> > > ---
> > > v2->v3:
> > > * Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
> >
> > I definitely didn't mean it done this way...
> >
> > [...]
> > > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > > b/drivers/net/ethernet/renesas/ravb.h
> > > index b980bce763d3..058aceac8c92 100644
> > > --- a/drivers/net/ethernet/renesas/ravb.h
> > > +++ b/drivers/net/ethernet/renesas/ravb.h
> > [...]
> > > @@ -965,6 +966,11 @@ enum CXR31_BIT {
> > > CXR31_SEL_LINK1 = 0x00000008,
> > > };
> > >
> > > +enum CXR35_BIT {
> > > + CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi
> > */
> >
> > No, please just declare:
>
>
> >
> > CXR35_HALFCYC_CLKSW = 0xffff0000,
>
> Q1) Why do you think we should use this value for setting MII?
>
> As per hardware manual the value you suggested is wrong for MII settings.
> See page 2157
>
> [A] The case which CXR35 SEL_XMII is used for the selection of RGMII/MII
> in APB Clock 100 MHz.
>
> (1) To use RGMII interface, Set ‘H’03E8 0000’ to this register.
> (2) To use MII interface, Set ‘H’03E8 0002’ to this register.
>
> >
> > > + CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used
> > */
> >
> > All the other register *enum*s are declared from LSB to MSB. The
> > comment is pretty self-obvious here, please remove it. And declare the
> > whole field too:
> >
> > CXR35_SEL_XMII = 0x00000003,
>
> Values 1 and 3 are reserved so we cannot use 3.
>
> I think the current patch holds good as per the hardware manual for
> selecting MII interface. Please recheck and correct me if it is wrong.
>
> Cheers,
> Biju
>
> > CXR35_SEL_XMII_RGMII = 0x00000000,
> > CXR35_SEL_XMII_MII = 0x00000002,
> >
> > [...]
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > > b/drivers/net/ethernet/renesas/ravb_main.c
> > > index b357ac4c56c5..9a0d06dd5eb6 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -540,7 +540,14 @@ 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, CXR35_HALFCYC_CLKSW1000 | CXR35_SEL_XMII_MII,
> > > + CXR35);
> >
> > ravb_write(ndev, (1000 << 16) | CXR35_SEL_XMII_MII, CXR35);
Oops. Missed this magic number changes. Will send v4 with above suggestions.
Cheers,
Biju
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
2022-09-14 17:20 ` Biju Das
2022-09-14 17:29 ` Biju Das
@ 2022-09-14 17:30 ` Sergey Shtylyov
2022-09-14 17:35 ` Biju Das
1 sibling, 1 reply; 6+ messages in thread
From: Sergey Shtylyov @ 2022-09-14 17:30 UTC (permalink / raw)
To: Biju Das, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Prabhakar Mahadev Lad, netdev, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das
On 9/14/22 8:20 PM, Biju Das wrote:
[...]
>>> 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>
>>> ---
>>> v2->v3:
>>> * Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
>>
>> I definitely didn't mean it done this way...
>>
>> [...]
>>> diff --git a/drivers/net/ethernet/renesas/ravb.h
>>> b/drivers/net/ethernet/renesas/ravb.h
>>> index b980bce763d3..058aceac8c92 100644
>>> --- a/drivers/net/ethernet/renesas/ravb.h
>>> +++ b/drivers/net/ethernet/renesas/ravb.h
>> [...]
>>> @@ -965,6 +966,11 @@ enum CXR31_BIT {
>>> CXR31_SEL_LINK1 = 0x00000008,
>>> };
>>>
>>> +enum CXR35_BIT {
>>> + CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi
>> */
>>
>> No, please just declare:
>
>
>>
>> CXR35_HALFCYC_CLKSW = 0xffff0000,
>
> Q1) Why do you think we should use this value for setting MII?
Where are you seeing me asking for that? This is just the field declaration,
correct against the manual... we can safely omit it as well...
[...]
>>> + CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used
>> */
>>
>> All the other register *enum*s are declared from LSB to MSB. The
>> comment is pretty self-obvious here, please remove it. And declare the
>> whole field too:
>>
>> CXR35_SEL_XMII = 0x00000003,
>
> Values 1 and 3 are reserved so we cannot use 3.
Again, this is the filed declaration, correct against the manual...
> I think the current patch holds good as per the hardware manual
> for selecting MII interface.
It is incomplete, compared against the manual. And declaring
CXR35_HALFCYC_CLKSW1000 just looks completely ridiculous. :-)
> Please recheck and correct me
> if it is wrong.
> Cheers,
> Biju
[...]
MBR, Sergey
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
2022-09-14 17:30 ` Sergey Shtylyov
@ 2022-09-14 17:35 ` Biju Das
0 siblings, 0 replies; 6+ messages in thread
From: Biju Das @ 2022-09-14 17:35 UTC (permalink / raw)
To: Sergey Shtylyov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Prabhakar Mahadev Lad, netdev, linux-renesas-soc,
Geert Uytterhoeven, Chris Paterson, Biju Das
Hi Sergey,
> Subject: Re: [PATCH net-next v3] ravb: Add RZ/G2L MII interface support
>
> On 9/14/22 8:20 PM, Biju Das wrote:
>
> [...]
> >>> 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>
> >>> ---
> >>> v2->v3:
> >>> * Documented CXR35_HALFCYC_CLKSW1000 and CXR35_SEL_XMII_MII macros.
> >>
> >> I definitely didn't mean it done this way...
> >>
> >> [...]
> >>> diff --git a/drivers/net/ethernet/renesas/ravb.h
> >>> b/drivers/net/ethernet/renesas/ravb.h
> >>> index b980bce763d3..058aceac8c92 100644
> >>> --- a/drivers/net/ethernet/renesas/ravb.h
> >>> +++ b/drivers/net/ethernet/renesas/ravb.h
> >> [...]
> >>> @@ -965,6 +966,11 @@ enum CXR31_BIT {
> >>> CXR31_SEL_LINK1 = 0x00000008,
> >>> };
> >>>
> >>> +enum CXR35_BIT {
> >>> + CXR35_HALFCYC_CLKSW1000 = 0x03E80000, /* 1000 cycle of clk_chi
> >> */
> >>
> >> No, please just declare:
> >
> >
> >>
> >> CXR35_HALFCYC_CLKSW = 0xffff0000,
> >
> > Q1) Why do you think we should use this value for setting MII?
>
> Where are you seeing me asking for that? This is just the field
> declaration, correct against the manual... we can safely omit it as
> well...
OK will keep it as field declaration and use actual value during setting.
>
> [...]
> >>> + CXR35_SEL_XMII_MII = 0x00000002, /* MII interface is used
> >> */
> >>
> >> All the other register *enum*s are declared from LSB to MSB. The
> >> comment is pretty self-obvious here, please remove it. And declare
> >> the whole field too:
> >>
> >> CXR35_SEL_XMII = 0x00000003,
> >
> > Values 1 and 3 are reserved so we cannot use 3.
>
> Again, this is the filed declaration, correct against the manual...
OK. Will add it.
>
> > I think the current patch holds good as per the hardware manual for
> > selecting MII interface.
>
> It is incomplete, compared against the manual. And declaring
> CXR35_HALFCYC_CLKSW1000 just looks completely ridiculous. :-)
Ok. All good now. Will send 4.
Cheers,
Biju
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-09-14 17:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-14 6:47 [PATCH net-next v3] ravb: Add RZ/G2L MII interface support Biju Das
2022-09-14 17:08 ` Sergey Shtylyov
2022-09-14 17:20 ` Biju Das
2022-09-14 17:29 ` Biju Das
2022-09-14 17:30 ` Sergey Shtylyov
2022-09-14 17:35 ` Biju Das
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).