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: <d20d3b73-38b4-fb06-2daa-125f446aeb44@kernel.org> Date: Mon, 28 Nov 2022 11:21:40 +0200 From: Roger Quadros <rogerq@...nel.org> To: Maciej Fijalkowski <maciej.fijalkowski@...el.com> Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com, pabeni@...hat.com, vigneshr@...com, linux-omap@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH v3 net-next 5/6] net: ethernet: ti: am65-cpsw: retain PORT_VLAN_REG after suspend/resume On 25/11/2022 15:21, Maciej Fijalkowski wrote: > On Wed, Nov 23, 2022 at 02:48:34PM +0200, Roger Quadros wrote: >> During suspend resume the context of PORT_VLAN_REG is lost so >> save it during suspend and restore it during resume for >> host port and slave ports. >> >> Signed-off-by: Roger Quadros <rogerq@...nel.org> >> Signed-off-by: David S. Miller <davem@...emloft.net> >> --- >> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 7 +++++++ >> drivers/net/ethernet/ti/am65-cpsw-nuss.h | 4 ++++ >> 2 files changed, 11 insertions(+) >> >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> index 0b59088e3728..f5357afde527 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c >> @@ -2875,7 +2875,9 @@ static int am65_cpsw_nuss_suspend(struct device *dev) >> struct am65_cpsw_port *port; >> struct net_device *ndev; >> int i, ret; >> + struct am65_cpsw_host *host_p = am65_common_get_host(common); > > Nit: I see that retrieving host pointer depends on getting the common > pointer first from dev_get_drvdata(dev) so pure RCT is not possible to > maintain here but nonetheless I would move this line just below the common > pointer: > > struct am65_cpsw_common *common = dev_get_drvdata(dev); > struct am65_cpsw_host *host = am65_common_get_host(common); > struct am65_cpsw_port *port; > struct net_device *ndev; > int i, ret; OK. > > Also I think plain 'host' for variable name is just fine, no need for _p > suffix to indicate it is a pointer. in that case you should go with > common_p etc. host_p is the naming convention used throughout the driver. Do think it is a good idea to change it at this one place? > >> >> + host_p->vid_context = readl(host_p->port_base + AM65_CPSW_PORT_VLAN_REG_OFFSET); >> for (i = 0; i < common->port_num; i++) { >> port = &common->ports[i]; >> ndev = port->ndev; >> @@ -2883,6 +2885,7 @@ static int am65_cpsw_nuss_suspend(struct device *dev) >> if (!ndev) >> continue; >> >> + port->vid_context = readl(port->port_base + AM65_CPSW_PORT_VLAN_REG_OFFSET); >> netif_device_detach(ndev); >> if (netif_running(ndev)) { >> rtnl_lock(); >> @@ -2909,6 +2912,7 @@ static int am65_cpsw_nuss_resume(struct device *dev) >> struct am65_cpsw_port *port; >> struct net_device *ndev; >> int i, ret; >> + struct am65_cpsw_host *host_p = am65_common_get_host(common); >> >> ret = am65_cpsw_nuss_init_tx_chns(common); >> if (ret) >> @@ -2941,8 +2945,11 @@ static int am65_cpsw_nuss_resume(struct device *dev) >> } >> >> netif_device_attach(ndev); >> + writel(port->vid_context, port->port_base + AM65_CPSW_PORT_VLAN_REG_OFFSET); >> } >> >> + writel(host_p->vid_context, host_p->port_base + AM65_CPSW_PORT_VLAN_REG_OFFSET); >> + >> return 0; >> } >> #endif /* CONFIG_PM_SLEEP */ >> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h >> index 2c9850fdfcb6..e95cc37a7286 100644 >> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h >> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h >> @@ -55,12 +55,16 @@ struct am65_cpsw_port { >> bool rx_ts_enabled; >> struct am65_cpsw_qos qos; >> struct devlink_port devlink_port; >> + /* Only for suspend resume context */ >> + u32 vid_context; >> }; >> >> struct am65_cpsw_host { >> struct am65_cpsw_common *common; >> void __iomem *port_base; >> void __iomem *stat_base; >> + /* Only for suspend resume context */ >> + u32 vid_context; >> }; >> >> struct am65_cpsw_tx_chn { >> -- >> 2.17.1 >> -- cheers, -roger
Powered by blists - more mailing lists