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

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

[Uwe Kleine-König]

There are no known reasons to not use this driver as a module, so allow
to configure PCIE_MICROCHIP_HOST=m.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/pci/controller/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index b8d96d38064d..6eae2289410a 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -301,7 +301,7 @@ config PCI_LOONGSON
 	  Loongson systems.
 
 config PCIE_MICROCHIP_HOST
-	bool "Microchip AXI PCIe host bridge support"
+	tristate "Microchip AXI PCIe host bridge support"
 	depends on PCI_MSI && OF
 	select PCI_MSI_IRQ_DOMAIN
 	select GENERIC_MSI_IRQ_DOMAIN

base-commit: 3123109284176b1532874591f7c81f3837bbdc17
prerequisite-patch-id: e8aad0ef8193038684bc2e10d387a7b74da1116a
-- 
2.35.1

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

[Pali Rohár]

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

> so allow to configure PCIE_MICROCHIP_HOST=m.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/pci/controller/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index b8d96d38064d..6eae2289410a 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -301,7 +301,7 @@ config PCI_LOONGSON
>  	  Loongson systems.
>  
>  config PCIE_MICROCHIP_HOST
> -	bool "Microchip AXI PCIe host bridge support"
> +	tristate "Microchip AXI PCIe host bridge support"
>  	depends on PCI_MSI && OF
>  	select PCI_MSI_IRQ_DOMAIN
>  	select GENERIC_MSI_IRQ_DOMAIN
> 
> base-commit: 3123109284176b1532874591f7c81f3837bbdc17
> prerequisite-patch-id: e8aad0ef8193038684bc2e10d387a7b74da1116a
> -- 
> 2.35.1
> 

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

[Conor.Dooley]

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?

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?
Not quite sure what the expectation is with attestation for something
like this.

Thanks,
Conor

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

[Pali Rohár]

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.

> 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.

> Not quite sure what the expectation is with attestation for something
> like this.
> 
> Thanks,
> Conor

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

[Conor.Dooley]

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.

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]

On Thu, Apr 21, 2022 at 01:55:01PM +0000, Conor.Dooley@microchip.com wrote:
> 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 :)

FTR: I don't have the hardware, I just touched the driver because I
found that missing ; that didn't hurt with the driver =y. Wondering why
it was bool was just the obvious next thought.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]