Hi Péter
Péter Ujfalusi <peter.ujfalusi@xxxxxxxxx> writes:
> On 26/09/2022 21:50, Kevin Hilman wrote:
>> Péter Ujfalusi <peter.ujfalusi@xxxxxxxxx> 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@xxxxxxxxxxxx>
>>>> ---
>>>> 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
