Re: [PATCH] PCI: microchip: Allow driver to be built as a module

[<Conor.Dooley@microchip.com>]

On 21/04/2022 13:41, Pali Rohár wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Thursday 21 April 2022 11:31:16 Conor.Dooley@microchip.com wrote:
>> On 20/04/2022 16:41, Pali Rohár wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Wednesday 20 April 2022 11:34:49 Uwe Kleine-König wrote:
>>>> There are no known reasons to not use this driver as a module,
>>>
>>> Hello! I think that there are reasons. pcie-microchip-host.c driver uses
>>> builtin_platform_driver() and not module_platform_driver(); it does not
>>> implement .remove driver callback and also has set suppress_bind_attrs
>>> to true. I think that all these parts should be properly implemented
>>> otherwise it does not have sane reasons to use driver as loadable and
>>> unloadable module.
>>>
>>> Btw, I implemented proper module support for pci-mvebu.c driver
>>> recently, so you can take an inspiration. See:
>>> https://lore.kernel.org/linux-pci/20211126144307.7568-1-pali@kernel.org/t/#u
>>
>> Hmm, so what is the way forward here, are you happy to do it yourself
>> or do you not have the hardware/would rather that we did it?
> 
> Hello! It would be needed to implement remove callback. But I do not
> have hardware for doing and testing it, so I do not feel that I can do
> it. I think that somebody with hardware and documentation should look at
> it and decide what is required to do in remove/cleanup procedure.
> 
> Also it would be needed to investigate if something more is needed to
> change builtin_platform_driver() to module_platform_driver(). If there
> are not some other steps which needs to be done in correct sequence and
> usage of builtin_platform_driver() currently ensures it.

Was more wondering if this was something Uwe had hardware for than
yourself, since he was poking around at the driver. But (assuming he
doesnt either) I'll add this to our todo :)

> 
>> If you'd prefer that we did it, do we change the driver & submit that
>> as a series with this patch as patch 2/2? Or should it be a single
>> patch with your suggested-by?
> 
> Feel free to put it into one patch. It is single change which implements
> one new feature = module support.

I probably should have specified, it'd be Uwe's suggested-by once merged
into a single patch.

Thanks,
Conor.