Date:   Thu, 16 Mar 2023 11:08:39 +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: <ZBLqp5rV48l1I+E+@corigine.com>
References: <20230315110546.2518305-1-msp@baylibre.com>
 <20230315110546.2518305-12-msp@baylibre.com>
 <ZBLndFdGKYApfBv4@corigine.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZBLndFdGKYApfBv4@corigine.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?VUlg7fDUUOIxWRYZSUwpYhxrKw6RtLd0OHkntQabWjMwNPmaIs3R1tCOjlDh?=
 =?us-ascii?Q?Dwcx4BIMTqy1KugkBgPsvzDC42YJufVkFFN2pNX5uiQOP8CzkionBAYYdIxY?=
 =?us-ascii?Q?DDMPVUPS337uFCqvQwV2ZvgXq7IMHofqlqb0VfZMG3UBQmY/CH4ZQ0/qoILA?=
 =?us-ascii?Q?PZTkYX0BXkLmNVPuTis7jpJB+A+EXuf9No0zaQ/ZSgaTdlvx2SZC7cYn56PX?=
 =?us-ascii?Q?kvc6/fEMYFLO/kcUbQWhAyegdH/wnLJITU+YZWrfjNNzLkKJ77009dD9uOMS?=
 =?us-ascii?Q?fc7gm9cvC0pEywZDuKL/jRHeBS8qQVTkuBzB64LrkZxVdqV4tAcLt7pQv4MT?=
 =?us-ascii?Q?1qzxZiW0QLpads6Zd0+QHwiCWYs0m/KIGydzvxJMSzYI1q27u9cYqSEmfIao?=
 =?us-ascii?Q?2TVkOgJynGVY1SEvum/QB/hMURMed/j/6s6QnuELSz/M/sy8IU7HM4zGskNP?=
 =?us-ascii?Q?W7vqHV7IwD6/ZniMFwNJEen3IJvp4+vsLJH9kNrGBoW0fRwZjWc0/zPtqCnt?=
 =?us-ascii?Q?PtsoAOxYyDkj1O3qYx3RXgezHQyZnPFZiep8VGkG0fs/XKMbARmJdimufVzT?=
 =?us-ascii?Q?gp+LI/KnCRgdrBZy6uo4UCDcEdQIBKfxb2m8btvcjCtUB56J7qnO8wKpbuwV?=
 =?us-ascii?Q?T4i9ni7pIFs6B2CIcPIQGMYViu7Za1+XOCx1youfonr3MVv8tX9foVezoRYJ?=
 =?us-ascii?Q?03mKFKM+MzwF70jNu8Aj7o2F7HYJgWUMGQX1DKrMIe9w333uzIlw6QaBCw1/?=
 =?us-ascii?Q?EwN3+u7M0EZ5sOJ1Ehhbufz7a+pMZqxhHbAGqlrN1hszBdhH3FZSlrj9ENN1?=
 =?us-ascii?Q?yY01jeZjIoeK8bTOswNjcBJp1z6XigdZXNxF3FNPe+TVIshYo4qI6ycZ3VV3?=
 =?us-ascii?Q?m94KMx4ovx4XRVVlFG8AbeS1zdJf/Y3afjekxmKDe3+4FVY/MCeyZkcSAeMS?=
 =?us-ascii?Q?rlDbWEtnZTuy0gCLq2P96c5zhk1nCcNCqRz6Xeh7WgV89a0q3FgMd1TCsrI3?=
 =?us-ascii?Q?eOSwZU84FOIUk5v/uZVNspz6ocYfQD7Y9E8YOlMZj7xfghiJeWmDGbwEy5vp?=
 =?us-ascii?Q?GAhYg87VRd2PLTEVFiBX70R1m4Ri21xwoIJNpSS6+mYMnx38NlwlATN6EPhX?=
 =?us-ascii?Q?4odLGY3NrYUKP19WkvqFCvP21bcZeeiPSlQEzhBSHLVNYhYBhJsdnwJkQekR?=
 =?us-ascii?Q?PUtzuWLWLXLQnJ+OE8nqq10/aBe6fTIWtOwWowZ4ZnwHDKQrRkdVFop1M94i?=
 =?us-ascii?Q?s/aL09Vdzqbj6qOiYRV+9iWyjPluPcrRlil3EE3OoTQDrw2spz1kJ5n4kOY6?=
 =?us-ascii?Q?PJykw6Av2P7aWbyWdVXt4Al0yAYB6l2c+YxJqi9fsm2kMLZgK8TypuXuDLox?=
 =?us-ascii?Q?zY97JqCnmxS7bdIde+0zqCOLSnn6bFDFC7qXuCO6hDG6mpTHDETiJyRv4Cj/?=
 =?us-ascii?Q?pIzdU6uLck4F6wRMDLGfAXvHVRkbXmZ6Z5Hn5rP4ZtL5iocS9tHaBPdGlYMS?=
 =?us-ascii?Q?LfT4k1KjQB+E4+QOC4bTpiOxDLE80Tdi7/BKOFPJLcLnQe7Rx3iRlLEqIBGP?=
 =?us-ascii?Q?uaCyD6w+x8tFWgia3Jzj2pmmjnEks3c8SHc+jr5UDbQgaAm90PFHPeR86eYT?=
 =?us-ascii?Q?MHJ0HK1J/ZKafhIQokYEJz20ZINqdq+x3l1YtOFz75AFMgMiiXHkHGZysBjI?=
 =?us-ascii?Q?OG2iLg=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 97c35db6-a17f-4593-feb1-08db26066dfb
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Mar 2023 10:08:46.2447
 (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: KVgGga/l0KpDFiuhfUp5hVjHdaMlWS28qBodTN6ex8XqZmswifOwpEq5O6VY/N4vGt/a7DaHlL2CYofvGq2SGOBR0aPUDKABdUQ4HYen1QM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR13MB6279
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Mar 16, 2023 at 10:55:00AM +0100, Simon Horman wrote:
> 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>

Nit in my previous email (which I hit send on a little too soon)
not withstanding,

Signed-off-by: Simon Horman <simon.horman@corigine.com>

FWIIW, I am taking a pause in my review now.

...

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