Re: [PATCH] amba: make amba_bustype constant

[Kunwu Chan <kunwu.chan@linux.dev>]

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