Re: [PATCH] amba: make amba_bustype constant

[Kunwu Chan]

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