On 9/1/2026 10:01 PM, Krzysztof Kozlowski wrote:
> On 01/09/2026 12:31, Linlin Zhang wrote:
>>
>>
>> On 8/31/2026 2:58 PM, Krzysztof Kozlowski wrote:
>>> On 27/08/2026 18:07, Linlin Zhang wrote:
>>>> +static void crypto_virt_remove(struct platform_device *pdev)
>>>> +{
>>>> + virtblk_set_crypto_ops(NULL);
>>>> +}
>>>> +
>>>> +static const struct of_device_id crypto_virt_of_match[] = {
>>>> + { .compatible = "qcom,crypto-virt" },
>>>
>>> Undocumented ABI, also not really allowed. We don't take bindings for
>>> drivers.
>>>
>>
>> The dt-binding document is included in another patch in this series.
>
> And what does the submitting patches document ask you?
>
>
>> The concern about introducing a DT binding for a software-only driver.
>> The underlying problem I am trying to solve is how to communicate the
>> wrapped-key size to the guest.
>
> I am not asking about your problem. Can you please stop answering to
> actual problems with irrelevant other aspects?
>
> You cannot have DT for a driver.
ACK
I'll drop the DT binding and this patch, and look for an alternative
mechanism to convey the wrapped-key size and key relevant requests.
>
> That's it, please read DTS101 slides.
>
>
>>
>> Because the guest has no access to the ICE hardware, it cannot get
>> the key size by reading the ICE hardware directly. In the other hand,
>> the wrapped key size may vary across ICE versions, it's infeasible
>> to have a fixed wrapped key size in the code. Additionally, wrapped
>> key size is vendor-specific, it's nor proper to pass it via virtio.
>>
>> As an experiment, I used a DT property to set this wrapped key and
>> wanted to get feedback on whether this is a reasonable approach.
>>
>> Is there a preferred mechanism for handling this kind of vendor-specific
>> configuration, or are there similar examples elsewhere in the kernel
>> that I should follow?
>
> Not DT. Use whatever other mechanism kernel provides.
>
>
>>
>>>> + { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, crypto_virt_of_match);
>>>> +
>>>> +static struct platform_driver crypto_virt_driver = {
>>>> + .probe = crypto_virt_probe,
>>>> + .remove = crypto_virt_remove,
>>>> + .driver = {
>>>> + .name = "crypto_virt",
>>>> + .of_match_table = crypto_virt_of_match,
>>>> + },
>>>> +};
>>>> +
>>>> +static int __init crypto_virt_init(void)
>>>> +{
>>>> + return platform_driver_register(&crypto_virt_driver);
>>>> +}
>>>> module_init(crypto_virt_init);
>>>>
>>>> #if IS_MODULE(CONFIG_QCOM_CRYPTO_VIRT)
>>>> static void __exit crypto_virt_exit(void)
>>>> {
>>>> - virtblk_set_crypto_ops(NULL);
>>>> + platform_driver_unregister(&crypto_virt_driver);
>>>
>>> You just added this line, so previous code was simply wrong?
>> No. In previous patch, virtblk_set_crypto_ops is called in
>> crypto_virt_init and crypto_virt_exit. This patch moves the caller
>> to the driver probe and remove APIs.
>
> Heh?
>
> Let's look at your previous patch:
>
> +#if IS_MODULE(CONFIG_QCOM_CRYPTO_VIRT)
> +static void __exit crypto_virt_exit(void)
> +{
> + virtblk_set_crypto_ops(NULL);
> +}
> +module_exit(crypto_virt_exit);
> +#endif
>
> And you claim that you did not add this line?
>
> Do you understand how diff format works?
>
> Best regards,
> Krzysztof
