On 31/08/2022 17:44, Tuma, Martin (Digiteq Automotive) wrote:
>
>
>> Ah, right, you do not use it for DT platform. Then you need proper ID
>> table, e.g. for ACPI. platform_device_id table would also do the trick
>> but I don't think it is suitable for such matching via ACPI.
>
> The mgb4 driver of course uses the propper device id table (the PCI id) and
> matches and loads fine. The problem is, it needs two other modules to be loaded
> prior to it, where one of them is the xiic module. It is used by a platform device
> that gets created/instantiated during the mgb4 inicialization. As there is no symbol
> dependency, the dependency between the modules can only be defined using
> MODULE_SOFTDEP. And for modprobe to work correctly you need the platform
> alias.
I don't know what is mgb4 - there is nothing like that in the sources
(git grep, find). Other existing devices instantiate it via MFD child
device, which works on platform devices and this points to the need of
platform_device_id table.
The commit could then indicate as fix for:
Fixes: b822039b8ec1 ("i2c: xiic: Fix coding style issues")
I also don't understand the reason for alias removal in that commit -
"none is really using it" - because at least one driver (timberdale)
uses it...
>
>>> The fact really is, that on x86_64 and ARM (Nvidia jetson) without any specific devicetree
>>> where I tested the driver, the mgb4 driver loads properly both the I2C and SPI modules
>>> defined using MODULE_SOFTDEP (there is no link dependency) if and only if they are
>>> defined using the "platform" prefix (and the module has that alias, hence this patch). So
>>> there must IMHO be some mechanism in the kernel or in modprobe, that works based
>>> on the prefix.
>>
>> Nvidia Jetson is ARM (and not an ACPI?) so it comes with DT. Let's don't
>> mix problems. Depending on the type of your system where this is used,
>> you need proper matching. Sprinkling aliases is not the way, usually.
>
> This is not problem mixing. You really can not expect every user to define a DT
> for a PCI Express card that he may or may not use! The type of the system is
> irrelevant here, a PCIe card has to work based on the PCI id and not some
> additional mechanism like DT or ACPI you suggest.
The type of system is relevant because from that piece you start
analyzing the problem. I have no clue which piece added such device in
your system (ACPI tables? DTB) and you failed to provide such information.
> The problem this patch is solving is the inter-module dependency (mgb4
> requires xiic to be loaded). If you think that this inter-module dependency should
> be solved differently, then please provide _how exactly_ this should be done,
I already said - proper device tables.
> not
> some hypotetic solutions for problems that we do not have like some platform
> dependency of the drivers, and I will rewrite the patches. Otherwise I really do not
> see any reason for your fight agains this one line patch, that adds an alias that
> many other drivers (like the second one we are using in mgb4 - the Xilinx SPI
> driver) already have and that actually solves the problem.
Aliases are hiding the actual user and binding method leading to commit
like b822039b8ec1 saying - no one uses it. You need to implement proper
matching method (e.g. platform device table), not sprinkle aliases. Just
because some other driver chosen poor way it is not argument to repeat it...
Best regards,
Krzysztof
