Re: [PATCH v2 06/10] soc: fsl: qe: Add support for QMC

Hi Christophe,
On Wed, 11 Jan 2023 13:58:03 +0000
Christophe Leroy [off-list ref] wrote:

Le 06/01/2023 à 17:37, Herve Codina a écrit :

quoted

The QMC (QUICC Multichannel Controller) is available on some
PowerQUICC SoC such as the MPC885 or MPC866.  

Same, that's QMC for CPM for the time being, should that be made clear 
in the subject ?

Also same answer:
Will changing the subject be enough or do I need to move it to new
created drivers/soc/fsl/cpm/ directory ?

QE also has QMC and this driver might be used for QE QMC as well with 
some enhancement, maybe that could be explained here ?

Yes and we plan to have it working on QE too.

What do you think if I add the following:
  The QMC is also available on some Quicc Engine SoC.
  This current version support CPM1 SoC only. Some
  enhancement are needed to support Quicc Engine SoC.

I see you are using in_be16(), out_be16() etc ....
That's specific to powerpc arch, maybe it is better to use ioread16be, 
iowrite16be() etc ....
See commit 3f39f38ea91d ("soc: fsl: qe: replace qe_io{read,write}* 
wrappers by generic io{read,write}*") and commit 6ac9b61786cc ("soc: 
fsl: qe: introduce qe_io{read,write}* wrappers") for information.

You're right.
I will switch to io{read,write}{16,32}be.

I prefer avoid the use of qe_* wrappers as the driver is not
(yet) compatible with QE.

quoted

It emulates up to 64 channels within one serial controller
using the same TDM physical interface routed from the TSA.

Signed-off-by: Herve Codina <herve.codina@bootlin.com>  

In some places, you comments use the network style, not the generic style.

I will fix them.

Christophe

Thanks for the review,
Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com