Re: [PATCH] amba: make amba_bustype constant

From: Kunwu Chan
Date: Tue Aug 27 2024 - 22:52:35 EST


Thanks for the reply.
On 2024/8/27 21:37, Andy Shevchenko wrote:
On Tue, Aug 27, 2024 at 03:45:31PM +0800, Kunwu Chan wrote:
On 2024/8/26 18:40, Andy Shevchenko wrote:
On Mon, Aug 26, 2024 at 06:08:11PM +0800, Kunwu Chan wrote:
On 2024/8/23 21:48, Andy Shevchenko wrote:
On Fri, Aug 23, 2024 at 02:42:03PM +0800, Kunwu Chan wrote:
...

-extern struct bus_type amba_bustype;
+extern const struct bus_type amba_bustype;
Can we actually hide this from the outside, i.e. make it static in the C file,
and introduce the dev_is_amba() helper instead?

P.S. You may look at the PNP bus case (some of the latest patches there)
I found it problematic in the process of making changes.

There have some usage of amba_bustype outside the AMBA code ,i.e:
https://elixir.bootlin.com/linux/v6.10.4/source/drivers/iommu/iommu.c#L155

So, If we make the amba_bustype inside the AMBA code, the outside cannot
access.
Yes, that's the idea.

If only internal access is allowed, it will compile and report an error, how
should I modify it next, and any suggestions?
(1) vvv

Make it patch series:
1) patch that introduces exported function called dev_is_amba() (1 patch)
Done.
2) convert user-by-user (N patches)

I've no idea about how to modify, such as in iommu.c:

staticconststructbus_type <https://elixir.bootlin.com/linux/v6.10.4/C/ident/bus_type>*constiommu_buses <https://elixir.bootlin.com/linux/v6.10.4/C/ident/iommu_buses>[]={
&platform_bus_type <https://elixir.bootlin.com/linux/v6.10.4/C/ident/platform_bus_type>,
#ifdef CONFIG_PCI <https://elixir.bootlin.com/linux/v6.10.4/K/ident/CONFIG_PCI>
&pci_bus_type <https://elixir.bootlin.com/linux/v6.10.4/C/ident/pci_bus_type>,
#endif
#ifdef CONFIG_ARM_AMBA <https://elixir.bootlin.com/linux/v6.10.4/K/ident/CONFIG_ARM_AMBA>
&amba_bustype <https://elixir.bootlin.com/linux/v6.10.4/C/ident/amba_bustype>, Or in arch/arm/mach-highbank/highbank.c#L150 bus_register_notifier <https://elixir.bootlin.com/linux/v6.10.4/C/ident/bus_register_notifier>(&amba_bustype <https://elixir.bootlin.com/linux/v6.10.4/C/ident/amba_bustype>,&highbank_amba_nb <https://elixir.bootlin.com/linux/v6.10.4/C/ident/highbank_amba_nb>);

3) hide the bus type and make it constant.

Done.

Move the 'extern struct' to drivers/amba/bus.c introduce some compilation errors.

(1) ^^^

Here[1] have many use of  amba_bustype directly outside the drivers/amba
Yes.

When I try to hide the amba_bustype by move the extern from
include/linux/amba to drivers/amba,

we find some errors: "error: use of undeclared identifier amba_bustype".
Yes, that's why I put (1) to how solve this as we usually do in the Linux
kernel.

I check the pnp_bus_type code, it did'nt use it the non-PNP code.

So I thought I should add a patch for dev_is_amba as your suggested and just leave it intact and submit it together.

--
Thanks,
Kunwu.Chan