Re: [PATCH 3/3] dma/ti: convert PSIL to be buildable as module

["Péter Ujfalusi" <peter.ujfalusi@gmail.com>]

On 26/09/2022 21:50, Kevin Hilman wrote:
> Péter Ujfalusi <peter.ujfalusi@gmail.com> writes:
> 
>> Hi Kevin,
>>
>> On 9/26/22 21:18, Kevin Hilman wrote:
>>> map symbols need EXPORT_SYMBOL and files need MODULE_LICENSE.
>>>
>>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>>> ---
>>>   drivers/dma/ti/Kconfig          | 3 ++-
>>>   drivers/dma/ti/k3-psil-am62.c   | 4 ++++
>>>   drivers/dma/ti/k3-psil-am64.c   | 4 ++++
>>>   drivers/dma/ti/k3-psil-am654.c  | 4 ++++
>>>   drivers/dma/ti/k3-psil-j7200.c  | 4 ++++
>>>   drivers/dma/ti/k3-psil-j721e.c  | 4 ++++
>>>   drivers/dma/ti/k3-psil-j721s2.c | 4 ++++
>>>   drivers/dma/ti/k3-psil.c        | 2 ++
>>>   8 files changed, 28 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma/ti/Kconfig b/drivers/dma/ti/Kconfig
>>> index f196be3b222f..2adc2cca10e9 100644
>>> --- a/drivers/dma/ti/Kconfig
>>> +++ b/drivers/dma/ti/Kconfig
>>> @@ -56,7 +56,8 @@ config TI_K3_UDMA_GLUE_LAYER
>>>   	  If unsure, say N.
>>>   
>>>   config TI_K3_PSIL
>>> -	bool
>>> +       tristate
>>> +       default TI_K3_UDMA
>>>   
>>>   config TI_DMA_CROSSBAR
>>>   	bool
>>> diff --git a/drivers/dma/ti/k3-psil-am62.c b/drivers/dma/ti/k3-psil-am62.c
>>> index 2b6fd6e37c61..7c4ca85f68b1 100644
>>> --- a/drivers/dma/ti/k3-psil-am62.c
>>> +++ b/drivers/dma/ti/k3-psil-am62.c
>>> @@ -4,6 +4,7 @@
>>>    */
>>>   
>>>   #include <linux/kernel.h>
>>> +#include <linux/module.h>
>>>   
>>>   #include "k3-psil-priv.h"
>>>   
>>> @@ -184,3 +185,6 @@ struct psil_ep_map am62_ep_map = {
>>>   	.dst = am62_dst_ep_map,
>>>   	.dst_count = ARRAY_SIZE(am62_dst_ep_map),
>>>   };
>>> +EXPORT_SYMBOL_GPL(am62_ep_map);
>>
>> Wouldn't it be better to build one module (k3-psil.ko) and link all the
>> platform libs into that?
>> They are unconditionally built in all cases anyways and makes the lsmod
>> under control.
>> And no need to export these maps at all is a plus.
> 
> I guess that's one option, but seems to be to be the wrong direction for
> a modular kernel.  To me, it seems like the next step would be to make
> it so only the SoC specific module is loaded instead of always loading
> them all.

The PSI-L map is a library atm and exporting all the maps outside of the 
PSI-L library is wrong. We shall have fixed API to look up (and update) 
a PSI-L endpoint configuration and only that API shall be allowed.

I prefer to have a single .ko binary for the PSI-L library/database for 
now. Optionally the individual SoC maps could be marked as init data and 
we could make a copy of the one that is needed on the booted device.

For SoC only loading this whole library way must be reworked to a 
platform or a bus driver (the bus description via DT was shot down 
during the initial UDMA submission, fyi). So you need to find a 'device' 
which would probe the PSI-L map and only load the map that is needed.

Furthermore: having the individual maps as separate .ko objects does not 
make much sense as none of them can be removed runtime, the symbols are 
used in the 'core' library.

-- 
Péter