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

[Kevin Hilman <khilman@baylibre.com>]

Hi Péter

Péter Ujfalusi <peter.ujfalusi@gmail.com> writes:

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

OK, I understand.  I'll send a v2 with everything built into a single
.ko (but I'll leave the initdata stuff for an optional follow-up series,
since I don't fully understand how/when all these maps are used.)

Thanks for your detailed review & suggestions,

Kevin