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@xxxxxxxxxx>
>> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>> ---
>> 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
