Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 25 Nov 2022 14:21:17 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To:     Roger Quadros <rogerq@kernel.org>
CC:     <davem@davemloft.net>, <kuba@kernel.org>, <edumazet@google.com>,
        <pabeni@redhat.com>, <vigneshr@ti.com>,
        <linux-omap@vger.kernel.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 net-next 5/6] net: ethernet: ti: am65-cpsw: retain
 PORT_VLAN_REG after suspend/resume
Message-ID: <Y4DBTbVxUpbJ5sEl@boxer>
References: <20221123124835.18937-1-rogerq@kernel.org>
 <20221123124835.18937-6-rogerq@kernel.org>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221123124835.18937-6-rogerq@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ls2NeYrEhkMofNjwin56dMF8jsZ+pPb0lLNqt/Mc5w7Wg7Lr2cZsCrZlYVh2?=
 =?us-ascii?Q?ytxm6K1w4mZeaDQZuEvg3oAeONsj6V2Yr1fcOABWlFKTU8cOSTiZb8ZEp0l7?=
 =?us-ascii?Q?mHKAaogOkLKIeXkgZJfbAjLTxoEkILEi1nPC5AeSl1mExZdRloEQKa6B/cbb?=
 =?us-ascii?Q?OFGhMFvGlNKcoPAwRgi7YSFnZMf2yS46kZ+Wk+v6OZ8ZRgTXOVObTiJwfGsW?=
 =?us-ascii?Q?c+JxROciw5zSpeT669FPhqtDt2FWNA8JunyhqLYMYycpPVS7O70xoWe08Tuo?=
 =?us-ascii?Q?1ua4kaBiv8fOGD/LFeRdLFZGZv4CtM18PmLsBsBLlIhmbCXFBz54l7FIXQ7P?=
 =?us-ascii?Q?kZW/g6glZKCpmhHXHC8xGXHChpH//MykmZ7f/s+IcnDihNYQVY860l6vs2ha?=
 =?us-ascii?Q?i6V6eTPp7ezZPOYxWPQASj4pL/hXJm4C1xTTqurFsysYSJLDpbjXBHBBXwYm?=
 =?us-ascii?Q?OiI70cQhIv0S2Eh2beaeMbJImzj6zcidSB5T/rORfQa4lIpK1S75ZBQAAEZz?=
 =?us-ascii?Q?mD2LqcGzIra1HFlc7Ncuk9G7eV39lwgEzy6dwgURUrp5d+jBMiltKMfi2lbA?=
 =?us-ascii?Q?cv/Q0+584RD4F9mlQKddP9MWn3j5ZWq8GMvRMEeG4utoMVWPf9mOT6LIaMxK?=
 =?us-ascii?Q?dw36N5iDsUBKfFoxfxyVv+PduyTvzinLzpk79mKBA0bm3kH7//7Yj11qMzUR?=
 =?us-ascii?Q?0790BC8PNgQcCVi8Vn/RpC5Oxkb0n9B5Jv2f0DpzUBKS+67+qSSZRfXk3Mu4?=
 =?us-ascii?Q?YfH91yyGUCqTkSEmFbbaPN6I8qcvSGlmwgXUucR/HcSeACKWURE2jo46EBX2?=
 =?us-ascii?Q?7q5Q3yx5qS7++6Clv3b66hgI21AZSa6/5a4bAsgtpQlSpRfasqE9IGvY4wdQ?=
 =?us-ascii?Q?qQfEp9vskYKwx4hWwNLM3Fui1NkqKvIHfoyf/hB1dPgbG7JpiKVGQtZg+US9?=
 =?us-ascii?Q?4qZgSqwNDjgzhkrlWIQvjoC3T3Oie+nT3fc+tzlfnSuKeTg1uaMmWYAGLqHl?=
 =?us-ascii?Q?IKuYQH6z/GzIQX+lxalILRzrBpRQg/OtEORVIgaeLCUTZSP6dfnSb1KOMw8u?=
 =?us-ascii?Q?6Qrx/5TzT8/FfPd0wHm+Zf5OF2tOyyA19LEQ3yTqOwFKv40CmZWnXscChlt1?=
 =?us-ascii?Q?w06Z5daIPOChgprihVDIBT+82PN54eXh3A01hApTrXMu7TEnU0p1s2wqWjQr?=
 =?us-ascii?Q?pzjo0bm8e1SqZQJx0IgCPJ5N7jmiBFJJf7pPlTu+6uNfuYoZmmHXZKRDpi8n?=
 =?us-ascii?Q?xpANUD7rDdbfsPr935TzF68x+GhresmmpiR8HZmkzbwFXWOhZYVaaH7w9WJs?=
 =?us-ascii?Q?++DCRoSn//bGNFzc7hSBm2jgfAsqbHAZcmqxlxKMiMD1d3dyrt4aGqBqm1m5?=
 =?us-ascii?Q?+jKhIiT+noklMZM8mHmu+Ap2DcFSQQdedyPVmPN8UkOpBWY6YbIBfHlLH5c5?=
 =?us-ascii?Q?JqXSqEDGraoYJm8nFuU/xH6mKd3cvPqQyHGR3Js+2GEEWhlOZQvJjAdTTXA6?=
 =?us-ascii?Q?Gc+AgXQ/sSboObJxJzKw/a79OnrQ27E0TbnVYV6D62KYk9+83UJIY71pdqea?=
 =?us-ascii?Q?n9Dm66ADJKofHVQesJvHt/UwR+VZdNEbIrO6AraibEJELv/4uLzTVl9Rm7x9?=
 =?us-ascii?Q?gw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5fd679a0-3c00-4dcb-9d77-08dacee7f2e4
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6117.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Nov 2022 13:21:23.5588
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: d1yf4DHJCnkJOOA6ddQmjTWsQKy8aDjUXY80/JINHOWgrHThCKrd1xLOmkw5Lb1hfYqxPM/WS1c5O8C/2T6nournkCvpjNe/r02s5giLIgg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4936
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_PASS,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@kernel.org>
> Signed-off-by: David S. Miller <davem@davemloft.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;

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->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
>