Re: [RFC][PATCH] crypto: caam - Add missing MODULE_ALIAS

[Claudius Heine <ch@denx.de>]

Hi,

On 2021-09-20 15:43, Marek Vasut wrote:
> On 9/17/21 7:30 PM, Krzysztof Kozlowski wrote:
>> On 17/09/2021 18:21, Krzysztof Kozlowski wrote:
>>> On 17/09/2021 16:44, Horia Geantă wrote:
>>>> On 9/17/2021 1:33 PM, Krzysztof Kozlowski wrote:
>>>>> On 17/09/2021 11:51, Horia Geantă wrote:
>>>>>> On 9/16/2021 5:06 PM, Marek Vasut wrote:
>>>>>>> On 9/16/21 3:59 PM, Krzysztof Kozlowski wrote:
>>>>>>>> On 16/09/2021 15:41, Marek Vasut wrote:
>>>>>>>>> Add MODULE_ALIAS for caam and caam_jr modules, so they can be 
>>>>>>>>> auto-loaded.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>>>>>>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>>>>>>>>> Cc: Horia Geantă <horia.geanta@nxp.com>
>>>>>>>>> Cc: Iuliana Prodan <iuliana.prodan@nxp.com>
>>>>>>>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>>>>>>>>> ---
>>>>>>>>>    drivers/crypto/caam/ctrl.c | 1 +
>>>>>>>>>    drivers/crypto/caam/jr.c   | 1 +
>>>>>>>>>    2 files changed, 2 insertions(+)
>>>>>>>>>
>>>>>>>>
>>>>>>>> Since you marked it as RFC, let me share a comment - would be 
>>>>>>>> nice to
>>>>>>>> see here explanation why do you need module alias.
>>>>>>>>
>>>>>>>> Drivers usually do not need module alias to be auto-loaded, 
>>>>>>>> unless the
>>>>>>>> subsystem/bus reports different alias than one used for binding. 
>>>>>>>> Since
>>>>>>>> the CAAM can bind only via OF, I wonder what is really missing 
>>>>>>>> here. Is
>>>>>>>> it a MFD child (it's one of cases this can happen)?
>>>>>>>
>>>>>>> I noticed the CAAM is not being auto-loaded on boot, and then I 
>>>>>>> noticed
>>>>>>> the MODULE_ALIAS fixes cropping up in the kernel log, but I couldn't
>>>>>>> find a good documentation for that MODULE_ALIAS. So I was hoping 
>>>>>>> to get
>>>>>>> a feedback on it.
>>>>>>>
>>>>>> What platform are you using?
>>>>>>
>>>>>> "make modules_install" should take care of adding the proper aliases,
>>>>>> relying on the MODULE_DEVICE_TABLE() macro in the caam, caam_jr 
>>>>>> drivers.
>>>>>>
>>>>>> modules.alias file should contain:
>>>>>> alias of:N*T*Cfsl,sec4.0C* caam
>>>>>> alias of:N*T*Cfsl,sec4.0 caam
>>>>>> alias of:N*T*Cfsl,sec-v4.0C* caam
>>>>>> alias of:N*T*Cfsl,sec-v4.0 caam
>>>>>> alias of:N*T*Cfsl,sec4.0-job-ringC* caam_jr
>>>>>> alias of:N*T*Cfsl,sec4.0-job-ring caam_jr
>>>>>> alias of:N*T*Cfsl,sec-v4.0-job-ringC* caam_jr
>>>>>> alias of:N*T*Cfsl,sec-v4.0-job-ring caam_jr
>>>>>
>>>>> Marek added a platform alias which is not present here on the list
>>>>> (because there are no platform device IDs). The proper question is who
>>>>> requests this device via a platform match? Who sends such event?
>>>>>
>>>> AFAICS the platform bus adds the "platform:" alias to uevent env.
>>>> in its .uevent callback - platform_uevent().
>>>>
>>>> When caam (platform) device is added, the uevent is generated with 
>>>> this env.,
>>>> which contains both OF-style and "platform:" modaliases.
>>>
>>> I am not saying about theoretical case, I know that platform bus will
>>> send platform uevent. How did this device end up in platform bus so this
>>> uevent is being sent? It should be instantiated from OF on for example
>>> amba bus or directly from OF FDT scanning.
>>>
>>> Therefore I have the same question - who requests device via a platform
>>> match? Is it used out-of-tree in different configuration?
>>
>> I tried to reproduce such situation in case of a board I have with me
>> (Exynos5422). I have a platform_driver only with of_device_id table. The
>> driver is built as module. In my DTS the device node is like
>> (exynos5.dtsi and device is modified exynos-chipid to be a module):
>>
>>         soc: soc {
>>                  compatible = "simple-bus";
>>                  #address-cells = <1>;
>>                  #size-cells = <1>;
>>                  ranges;
>>
>>                  chipid: chipid@10000000 {
>>                          compatible = "samsung,exynos4210-chipid";
>>                          reg = <0x10000000 0x100>;
>>                  };
>>
>>         ...
>>     };
>>
>> The module was properly autoloaded (via OF aliases/events) and device
>> was matched.
> 
> Please put this on hold for a bit, I need a colleague to check the udev 
> event on this platform before we can move on any further.

Here are the uevent entries without this RFC patch applied:

```
# udevadm info -q all -p devices/platform/soc@0/30800000.bus/30900000.crypto
P: /devices/platform/soc@0/30800000.bus/30900000.crypto
L: 0
E: DEVPATH=/devices/platform/soc@0/30800000.bus/30900000.crypto
E: DRIVER=caam
E: OF_NAME=crypto
E: OF_FULLNAME=/soc@0/bus@30800000/crypto@30900000
E: OF_COMPATIBLE_0=fsl,sec-v4.0
E: OF_COMPATIBLE_N=1
E: MODALIAS=of:NcryptoT(null)Cfsl,sec-v4.0
E: SUBSYSTEM=platform
E: USEC_INITIALIZED=4468986
E: ID_PATH=platform-30900000.crypto
E: ID_PATH_TAG=platform-30900000_crypto
```

regards,
Claudius