diff mbox series

[v4,9/9] drm/bridge: tfp410: Add platform module alias

Message ID 20240422191903.255642-10-sui.jingfeng@linux.dev (mailing list archive)
State New, archived
Headers show
Series drm/bridge: Allow using fwnode API to get the next bridge | expand

Commit Message

Sui Jingfeng April 22, 2024, 7:19 p.m. UTC
Otherwise when compiled as module, this driver will not be probed on
non-DT environment. This is a fundamential step to make this driver
truely OF-independent.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/bridge/ti-tfp410.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski April 23, 2024, 8:05 a.m. UTC | #1
On 22/04/2024 21:19, Sui Jingfeng wrote:
> Otherwise when compiled as module, this driver will not be probed on
> non-DT environment. This is a fundamential step to make this driver
> truely OF-independent.

NAK.

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.

Just check your aliases and look what is there. You already have
i2c:tfp410 alias. If you need platform one, for some reason, explain
what is your matching path and add appropriate ID table. With that
explanation, of course.

Best regards,
Krzysztof
Sui Jingfeng April 23, 2024, 10:12 a.m. UTC | #2
Hi,

On 2024/4/23 16:05, Krzysztof Kozlowski wrote:
> On 22/04/2024 21:19, Sui Jingfeng wrote:
>> Otherwise when compiled as module, this driver will not be probed on
>> non-DT environment. This is a fundamential step to make this driver
>> truely OF-independent.
> NAK.


:( ...


>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong (e.g. misses either
> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
> for incomplete ID table.


I think I could give you a reason.

1) When compile this driver into linux kernel

This driver will be probed if we create a platform device manually with the name "tfp410".
This is also true for the display-connector driver(drivers/gpu/drm/bridge/display-connector.c),
see patch 0005 of this series  and the simple-bridge driver(drivers/gpu/drm/bridge/simple-bridge.c)
see patch 0003 of this series.

2) But when compile this driver as module, creating a platform device manually with the same name
*won't* make those platform driver probed. I think, this is *inconsistent behavior*. Therefore, I
add MODULE_ALIAS() to keep the behavior consistent. That is, a driver, should be able to be probed
regardless it is compile as a kernel module or it is built into the kernel.


> Just check your aliases and look what is there. You already have
> i2c:tfp410 alias.

Right, but the i2c:tfp410 alias only guarantee the driver can be probed successfully
when tfp410 working as I2C slave. tfp410 can also works as a transparent bridge.


>   If you need platform one, for some reason, explain
> what is your matching path and add appropriate ID table. With that
> explanation, of course.

When tfp410 works as a transparent bridge. The device itself is just a platform device.
similar with the display-connector.c and simple-bridge.c.

It is not discoverable by the system on non-DT environment, this is the root problem.
I said the various display bridges drivers are fully DT dependent, Dimtry didn't agree!

He said "I can not agree here. It doesn't depend on DT." and then asks me to developing
some other solution witch could preserve code sharing. So here it is.


> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 23, 2024, 10:20 a.m. UTC | #3
On 23/04/2024 12:12, Sui Jingfeng wrote:
> Hi,
> 
> On 2024/4/23 16:05, Krzysztof Kozlowski wrote:
>> On 22/04/2024 21:19, Sui Jingfeng wrote:
>>> Otherwise when compiled as module, this driver will not be probed on
>>> non-DT environment. This is a fundamential step to make this driver
>>> truely OF-independent.
>> NAK.
> 
> 
> :( ...
> 
> 
>>
>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>> usually it means your device ID table is wrong (e.g. misses either
>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>> for incomplete ID table.
> 
> 
> I think I could give you a reason.
> 
> 1) When compile this driver into linux kernel
> 
> This driver will be probed if we create a platform device manually with the name "tfp410".

Then do not create devices manually. This is not y2000 to use board files.

> This is also true for the display-connector driver(drivers/gpu/drm/bridge/display-connector.c),
> see patch 0005 of this series  and the simple-bridge driver(drivers/gpu/drm/bridge/simple-bridge.c)
> see patch 0003 of this series.

They have the same problem.

> 
> 2) But when compile this driver as module, creating a platform device manually with the same name
> *won't* make those platform driver probed. I think, this is *inconsistent behavior*. Therefore, I
> add MODULE_ALIAS() to keep the behavior consistent. That is, a driver, should be able to be probed
> regardless it is compile as a kernel module or it is built into the kernel.
> 

That's obvious. Please focus on the actual issue here.

> 
>> Just check your aliases and look what is there. You already have
>> i2c:tfp410 alias.
> 
> Right, but the i2c:tfp410 alias only guarantee the driver can be probed successfully
> when tfp410 working as I2C slave. tfp410 can also works as a transparent bridge.

So which bus or driver instantiates the device? What use case is this?

> 
> 
>>   If you need platform one, for some reason, explain
>> what is your matching path and add appropriate ID table. With that
>> explanation, of course.
> 
> When tfp410 works as a transparent bridge. The device itself is just a platform device.
> similar with the display-connector.c and simple-bridge.c.
> 
> It is not discoverable by the system on non-DT environment, this is the root problem.
> I said the various display bridges drivers are fully DT dependent, Dimtry didn't agree!
> 
> He said "I can not agree here. It doesn't depend on DT." and then asks me to developing
> some other solution witch could preserve code sharing. So here it is.


You wrote long message without actually reading my answer early. I
already gave you the solution. Address that one.

Best regards,
Krzysztof
Sui Jingfeng April 23, 2024, 10:44 a.m. UTC | #4
On 2024/4/23 18:20, Krzysztof Kozlowski wrote:
> On 23/04/2024 12:12, Sui Jingfeng wrote:
>> Hi,
>>
>> On 2024/4/23 16:05, Krzysztof Kozlowski wrote:
>>> On 22/04/2024 21:19, Sui Jingfeng wrote:
>>>> Otherwise when compiled as module, this driver will not be probed on
>>>> non-DT environment. This is a fundamential step to make this driver
>>>> truely OF-independent.
>>> NAK.
>>
>> :( ...
>>
>>
>>> You should not need MODULE_ALIAS() in normal cases. If you need it,
>>> usually it means your device ID table is wrong (e.g. misses either
>>> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
>>> for incomplete ID table.
>>
>> I think I could give you a reason.
>>
>> 1) When compile this driver into linux kernel
>>
>> This driver will be probed if we create a platform device manually with the name "tfp410".
> Then do not create devices manually. This is not y2000 to use board files.


Not exactly, creating devices manually can be modern and fancy approach.
Complex driver need to create devices manually to paper over the issue(-EPROBE_DEFER)
raised with cross drivers subsystem design. Or for the purpose of better modularization.
See etnaviv, vkms, efifb, aux-bridge, aux-bus, ect.

OK, I know what you means here.

>> This is also true for the display-connector driver(drivers/gpu/drm/bridge/display-connector.c),
>> see patch 0005 of this series  and the simple-bridge driver(drivers/gpu/drm/bridge/simple-bridge.c)
>> see patch 0003 of this series.
> They have the same problem.
>
>> 2) But when compile this driver as module, creating a platform device manually with the same name
>> *won't* make those platform driver probed. I think, this is *inconsistent behavior*. Therefore, I
>> add MODULE_ALIAS() to keep the behavior consistent. That is, a driver, should be able to be probed
>> regardless it is compile as a kernel module or it is built into the kernel.
>>
> That's obvious. Please focus on the actual issue here.
>
>>> Just check your aliases and look what is there. You already have
>>> i2c:tfp410 alias.
>> Right, but the i2c:tfp410 alias only guarantee the driver can be probed successfully
>> when tfp410 working as I2C slave. tfp410 can also works as a transparent bridge.
> So which bus or driver instantiates the device? What use case is this?
>
>>
>>>    If you need platform one, for some reason, explain
>>> what is your matching path and add appropriate ID table. With that
>>> explanation, of course.
>> When tfp410 works as a transparent bridge. The device itself is just a platform device.
>> similar with the display-connector.c and simple-bridge.c.
>>
>> It is not discoverable by the system on non-DT environment, this is the root problem.
>> I said the various display bridges drivers are fully DT dependent, Dimtry didn't agree!
>>
>> He said "I can not agree here. It doesn't depend on DT." and then asks me to developing
>> some other solution witch could preserve code sharing. So here it is.
>
> You wrote long message without actually reading my answer early. I
> already gave you the solution. Address that one.

Use MODULE_DEVICE_TABLE() instead? OK, I understand then. Thanks a lot 
for the education.


> Best regards,
> Krzysztof
>
Krzysztof Kozlowski April 23, 2024, 10:49 a.m. UTC | #5
On 23/04/2024 12:44, Sui Jingfeng wrote:
>>>>    If you need platform one, for some reason, explain
>>>> what is your matching path and add appropriate ID table. With that
>>>> explanation, of course.
>>> When tfp410 works as a transparent bridge. The device itself is just a platform device.
>>> similar with the display-connector.c and simple-bridge.c.
>>>
>>> It is not discoverable by the system on non-DT environment, this is the root problem.
>>> I said the various display bridges drivers are fully DT dependent, Dimtry didn't agree!
>>>
>>> He said "I can not agree here. It doesn't depend on DT." and then asks me to developing
>>> some other solution witch could preserve code sharing. So here it is.
>>
>> You wrote long message without actually reading my answer early. I
>> already gave you the solution. Address that one.
> 
> Use MODULE_DEVICE_TABLE() instead? OK, I understand then. Thanks a lot 
> for the education.


Yes, at least for something which is real driver.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
index 58dc7492844f..6a0a0bbe3c6b 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -504,3 +504,4 @@  module_exit(tfp410_module_exit);
 MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
 MODULE_DESCRIPTION("TI TFP410 DVI bridge driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:tfp410");