Re: [PATCH v2 02/10] soc: fsl: qe: Add support for TSA

[Christophe Leroy]

Le 11/01/2023 à 16:06, Herve Codina a écrit :
> Hi Christophe,
> 
> On Wed, 11 Jan 2023 13:47:23 +0000
> Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
> 
>> Le 06/01/2023 à 17:37, Herve Codina a écrit :
>>> The TSA (Time Slot Assigner) is available in some
>>> PowerQUICC SoC such as the MPC885 or MPC866.
>>>
>>> Its purpose is to route some TDM time-slots to other
>>> internal serial controllers.
>>
>> Is the subject correct ? As far as I understand this patch adds support
>> for the TSA on the CPM (exactly on CPM1), not on the QE.
> 
> Yes exactly, it is a CPM1 support (kind of previous version of QE).
> 
> Do you think that fixing the subject is enough or do I need also
> to create a new directory drivers/soc/fsl/cpm/ and move these drivers
> (TSA and QMC) in this new directory.

I think fixing the subject should be good enough.

Today we already have in that drivers/soc/fsl/qe/ directory some code 
which is common between CPM and QE, mainly in qe_common.c

Not sure qe_common.c is the best name ever, but it has been given that 
name in commit 1291e49e8937 ("QE/CPM: move muram management functions to 
qe_common")

So let's things as they are, just be precise in the subject, and if the 
final intention is to have some TSA common to CPM and QE, let's just 
call it tsa.c . If it's dedicated to CPM at the end, then maybe call it 
cpm-tsa.c.

Maybe one day we should rename drivers/soc/fsl/qe/ to 
drivers/soc/fsl/cpm-qe/

> 
> The alternative could be to leave this driver in drivers/soc/qe/ and
> rename it to cpm-tsa.c.
> 
> For information, we have some plan to have this driver working
> with QE (not done yet).
> 
>>
>> By the way, there are already some embryo for handling TSA on QE in
>> drivers/soc/fsl/qe/qe-tdm.c
> 
> Yes but this can be seen as an extension only used by
> drivers/net/wan/fsl_ucc_hdlc.c and it supports QE only.
> 
> Not sure that qe-tdm.c will fit well if several other
> drivers instances use it.

Fair enough.

> 
>>
>>>
>>> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>
>>
>> Christophe
> 
> Thanks for the review,
> Hervé
>