lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <OS0PR01MB592297F89124DD62211582AE86469@OS0PR01MB5922.jpnprd01.prod.outlook.com> Date: Wed, 14 Sep 2022 17:20:13 +0000 From: Biju Das <biju.das.jz@...renesas.com> To: Sergey Shtylyov <s.shtylyov@....ru>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com> CC: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, Geert Uytterhoeven <geert+renesas@...der.be>, Chris Paterson <Chris.Paterson2@...esas.com>, Biju Das <biju.das@...renesas.com> 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@...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
Powered by blists - more mailing lists