linux-kernel - Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3601a374-4161-40e1-8a80-9bbfdae5bd8a@linaro.org>
Date: Sun, 10 Mar 2024 21:35:36 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Karel Balej <karelb@...li.ms.mff.cuni.cz>,
 Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
 Duje Mihanović <duje.mihanovic@...le.hr>,
 ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org
Subject: Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886
 PMIC

On 10/03/2024 12:35, Karel Balej wrote:
> Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
>> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
>>> Dmitry,
>>>
>>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
>>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
>>>>> From: Karel Balej <balejk@...fyz.cz>
>>>>>
>>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
>>>>> driver to handle it. The driver currently only provides a basic support
>>>>> omitting additional functions found in the vendor version, such as long
>>>>> onkey and GPIO integration.
>>>>>
>>>>> Signed-off-by: Karel Balej <balejk@...fyz.cz>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>>     RFC v3:
>>>>>     - Drop wakeup-source.
>>>>>     RFC v2:
>>>>>     - Address Dmitry's feedback:
>>>>>       - Sort includes alphabetically.
>>>>>       - Drop onkey->irq.
>>>>>       - ret -> err in irq_handler and no initialization.
>>>>>       - Break long lines and other formatting.
>>>>>       - Do not clobber platform_get_irq error.
>>>>>       - Do not set device parent manually.
>>>>>       - Use input_set_capability.
>>>>>       - Use the wakeup-source DT property.
>>>>>       - Drop of_match_table.
>>>>
>>>> I only said that you should not be using of_match_ptr(), but you still
>>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
>>>> proper module loading support.
>>>
>>> I removed of_match_table because I no longer need compatible for this --
>>> there are no device tree properties and the driver is being instantiated
>>> by the MFD driver.
>>>
>>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
>>> compiled as module? If that is the case, given what I write above, am I
>>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
>>> to use here?
>>
>> Yes, if uevent generated for the device is "platform:<name>" then
>> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
>> sets it up (OF modalias or platform), but you should be able to check
>> the format looking at the "uevent" attribute for your device in sysfs
>> (/sys/devices/bus/platform/...). 
> 
> The uevent is indeed platform.
> 
> But since there is only one device, perhaps having a device table is
> superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> fitting?

Adding aliases for standard IDs and standard cases is almost never
correct. If you need module alias, it means your ID table is wrong (or
missing, which is usually wrong).

> 
> Although I don't understand why this is even necessary when the driver
> name is such and the module is registered using
> `module_platform_driver`...

ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
Just run `modinfo`.

Best regards,
Krzysztof