Date:   Thu, 16 Mar 2023 10:55:00 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Markus Schneider-Pargmann <msp@baylibre.com>
Cc:     Marc Kleine-Budde <mkl@pengutronix.de>,
        Chandrasekar Ramakrishnan <rcsekar@samsung.com>,
        Wolfgang Grandegger <wg@grandegger.com>,
        Vincent MAILHOL <mailhol.vincent@wanadoo.fr>,
        linux-can@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 11/16] can: m_can: Cache tx putidx
Message-ID: <ZBLndFdGKYApfBv4@corigine.com>
References: <20230315110546.2518305-1-msp@baylibre.com>
 <20230315110546.2518305-12-msp@baylibre.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230315110546.2518305-12-msp@baylibre.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?I/OV+NkdNYrO9DsNUWStILFRbg4mIfMbzbnEJNBIe0cXKkg+NUznXIeIqMZw?=
 =?us-ascii?Q?iwcoQk6QVpJDwCRr5hOJcwsPDg0nmDzoWppcCVPLQdYW3ReNGwYLdoPgbVHc?=
 =?us-ascii?Q?pPxYjbPlrl3D5ClaHFP3JS4+3+iEp1YXv5Q8iXrY5JxHcguGJebRXHSnU3cR?=
 =?us-ascii?Q?ImY/UiLMU5PQUjmGvhGTpJjm7aDEp9kNTSgP+v6rdxmI+2rJxF2L8hwGlV08?=
 =?us-ascii?Q?7ol6GYEIweugy41k44IiFRHRHvasApee+zRYv4FKmYgs2fOB9lsx2tXybhbp?=
 =?us-ascii?Q?k3F/cWeZ+SzTFpAEH2Cue49VAw5Ut+RsIFjgKvqf2ZQOwcn1uuF08BkZ75dR?=
 =?us-ascii?Q?4xDuQ3rakkdwo4NA/rrVnTgKruQCm0boj3za0CM+yZyGvGqajBReSnaYhNdW?=
 =?us-ascii?Q?KSZSmM+g7JyvB/Gdto4Ac7oKmIDFQ2DHD3Mhd930JNfpSikmRsS8Z0JQYKuX?=
 =?us-ascii?Q?g1UjH8kGcGWEdsDebiCMGav2lpD8NkGlIu2C3DuqXASmECjiYltpbEmfiCx7?=
 =?us-ascii?Q?gMQfXnRtudH9Wg6E1bsOdtwihgMWRzQ8DhOqTejUhCtWOX6R+MuhDmZnQJhQ?=
 =?us-ascii?Q?X6hrqp5QsIkBTEi4S2hflctb6ZWhIwYLd0XjbqRWoFkexllhCeLv9shcvmLb?=
 =?us-ascii?Q?3dm1UcyL2I1VZZuqpWK6HOzq8aCurUqhXWAMMUy/+RMmii/eAUYC97aUpHk/?=
 =?us-ascii?Q?fdysBLUGpyyxMLZ3skehJxA0c//ItybM78iqUa1lE/ZXC62bvtLbZZTw4ZqT?=
 =?us-ascii?Q?8Mu5ogCxVPDqsattUtvR6wxtrdlmiH1WyTkUdx+qKFcTb/EKbE4S7Qen5Q1B?=
 =?us-ascii?Q?uUaG2riRZ/xI6vQ0CB72fLsdF0fqhd4H9y8b7oci2l/S05amn0ig6HPae62E?=
 =?us-ascii?Q?Y5BQRJcwFofKnH75LpxKkBwImSM8HQHS66TCMpm942taalK6nph2P93NzQJx?=
 =?us-ascii?Q?o50q3EFHvS3BGdclRX440Rdz/BtRm+kfaNZZn3/pZt/kI4DwHMN7349BZbgn?=
 =?us-ascii?Q?pDycm9iwByo4GWjpv7y1oH2qqxpHsZpdH327aUkMVFoqj+JyrvuZOfczs1bc?=
 =?us-ascii?Q?fHG+mBUI1hEa4u/ToC90jrKZyr2abIiT3Dd+phYwGk6w0/ypnc9PETZa6xiC?=
 =?us-ascii?Q?xVtgAH+8nIMHwLO2pm2X04zMEcf9BnOxo/NSVo8QZJ+MstxkemduHdK9EAjl?=
 =?us-ascii?Q?iflGhmZxxNJuYIOFa7IJ6ViAuwu2NttHtsgtr8U3eePqIpITSMk1CN2d4EA/?=
 =?us-ascii?Q?bFIFKmralIvCT8rAnfFNhurOIPge3SIF+TsCVldExurfjjAZGxR+0jA0EmIn?=
 =?us-ascii?Q?G+8qDF3VLw/U5uBOfL4S8l9O/hOyObcVQZ90Pwhp06U5NlB1lR+/wJFBJgJU?=
 =?us-ascii?Q?rjhD/sdhA5fqqVBC04yI3QTYyrgQPicjbYEZO1rHGhO/LSAhTvEzkqg3jHKg?=
 =?us-ascii?Q?zmbsKIbDPkZVV80G4d305PPYPlHU7OmBFsUbkfheKl257zuzxUeyaEnktZs4?=
 =?us-ascii?Q?94oPDGV32AJOQHjlZAa+5xCW4+dP/0dXdIUzK4APFSnMalyYYPMvJdOXDTRx?=
 =?us-ascii?Q?SRb33T+6aoE3MEy4ZZvrRFlhGb5JArM10RZzFXjFfu8KfJ9s91klMDuFcv5e?=
 =?us-ascii?Q?HbFn/Jai2xGUZRMdhI6p7QNNNEpdKZXmlRV/bzPgg9KUw5M7BzCboFgt+p/H?=
 =?us-ascii?Q?BtIRvg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 77c9502f-d562-441f-2d21-08db2604866b
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Mar 2023 09:55:08.1514
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: IfPrtNtZ4avCmlWom9+nWdjC/MDM6nhFY4+tGfruOkm1E2fDi2eq2OvbHZCPskiKkwd+Kc96c3rlxh3dXb05RJv+8Q0UMoqJp1fixzXR/jE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR13MB4521
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Mar 15, 2023 at 12:05:41PM +0100, Markus Schneider-Pargmann wrote:
> m_can_tx_handler is the only place where data is written to the tx fifo.
> We can calculate the putidx in the driver code here to avoid the
> dependency on the txfqs register.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/net/can/m_can/m_can.c | 8 +++++++-
>  drivers/net/can/m_can/m_can.h | 3 +++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 4e794166664a..d5bcce948d2c 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1475,6 +1475,10 @@ static int m_can_start(struct net_device *dev)
>  
>  	m_can_enable_all_interrupts(cdev);
>  
> +	if (cdev->version > 30)
> +		cdev->tx_fifo_putidx = FIELD_GET(TXFQS_TFQPI_MASK,
> +						 m_can_read(cdev, M_CAN_TXFQS));
> +
>  	return 0;
>  }
>  
> @@ -1765,7 +1769,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
>  		}
>  
>  		/* get put index for frame */
> -		putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);
> +		putidx = cdev->tx_fifo_putidx;
>  
>  		/* Construct DLC Field, with CAN-FD configuration.
>  		 * Use the put index of the fifo as the message marker,
> @@ -1798,6 +1802,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
>  
>  		/* Enable TX FIFO element to start transfer  */
>  		m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
> +		cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
> +					0 : cdev->tx_fifo_putidx);
>  
>  		/* stop network queue if fifo full */
>  		if (m_can_tx_fifo_full(cdev) ||
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index d0c21eddb6ec..548ae908ac4e 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -102,6 +102,9 @@ struct m_can_classdev {
>  	u32 tx_max_coalesced_frames_irq;
>  	u32 tx_coalesce_usecs_irq;
>  
> +	// Store this internally to avoid fetch delays on peripheral chips
> +	int tx_fifo_putidx;

nit: it might be slightly nicer to do a pass over the code
     and make putidx unsigned - assuming it is an unsigned value.

> +
>  	struct mram_cfg mcfg[MRAM_CFG_NUM];
>  };
>  
> -- 
> 2.39.2
>