| Message ID | e00498fccf6dcbcf63bd24a137bfc7abd1b98345.1716967720.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mailbox: ARM_MHU_V3 should depend on ARM64 | expand |
On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > dependency on ARM64, to prevent asking the user about this driver when > configuring a kernel for a different architecture than ARM64. > Hi, the ARM64 dependency was dropped on purpose after a few iterations of this series since, despite this being an ARM IP, it has really no technical dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a platform driver, so it seemed an uneeded artificial restriction to impose... ...having said that, surely my live testing were performed only on arm64 models as of now. So, I am not saying that I am against this proposed fix but what is the issue that is trying to solve, have you seen any compilation error ? or is it just to avoid the user-prompting ? Thanks, Cristian > Fixes: ca1a8680b134b5e6 ("mailbox: arm_mhuv3: Add driver") > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > The MHUv3 documentation is unclear about whether this can be present > only on ARM64, or also on ARM. > Googling 'site:arm.com MHUv3 "aarch32"' shows no results. > Googling 'site:arm.com MHUv3 "aarch64"' does show results. > --- > drivers/mailbox/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig > index 3b8842c4a3401579..8d4d1cbb1d4ca625 100644 > --- a/drivers/mailbox/Kconfig > +++ b/drivers/mailbox/Kconfig > @@ -25,6 +25,7 @@ config ARM_MHU_V2 > > config ARM_MHU_V3 > tristate "ARM MHUv3 Mailbox" > + depends on ARM64 || COMPILE_TEST > depends on HAS_IOMEM || COMPILE_TEST > depends on OF > help > -- > 2.34.1 >
Hi Cristian, On Wed, May 29, 2024 at 12:13 PM Cristian Marussi <cristian.marussi@arm.com> wrote: > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > dependency on ARM64, to prevent asking the user about this driver when > > configuring a kernel for a different architecture than ARM64. > > the ARM64 dependency was dropped on purpose after a few iterations of > this series since, despite this being an ARM IP, it has really no technical > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > platform driver, so it seemed an uneeded artificial restriction to impose... > ...having said that, surely my live testing were performed only on arm64 models > as of now. For that, we have COMPILE_TEST=y. > So, I am not saying that I am against this proposed fix but what is the > issue that is trying to solve, have you seen any compilation error ? or > is it just to avoid the user-prompting ? I did not see a compile error (I didn't enable it on any non-ARM platform). But it is rather futile to ask the user about (thousands of) drivers for hardware that cannot possibly be present on the system he is configuring a kernel for. > > --- a/drivers/mailbox/Kconfig > > +++ b/drivers/mailbox/Kconfig > > @@ -25,6 +25,7 @@ config ARM_MHU_V2 > > > > config ARM_MHU_V3 > > tristate "ARM MHUv3 Mailbox" > > + depends on ARM64 || COMPILE_TEST > > depends on HAS_IOMEM || COMPILE_TEST > > depends on OF > > help Gr{oetje,eeting}s, Geert
On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > Hi Cristian, > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > <cristian.marussi@arm.com> wrote: > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > dependency on ARM64, to prevent asking the user about this driver when > > > configuring a kernel for a different architecture than ARM64. > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > this series since, despite this being an ARM IP, it has really no technical > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > platform driver, so it seemed an uneeded artificial restriction to impose... > > ...having said that, surely my live testing were performed only on arm64 models > > as of now. > > For that, we have COMPILE_TEST=y. > > > So, I am not saying that I am against this proposed fix but what is the > > issue that is trying to solve, have you seen any compilation error ? or > > is it just to avoid the user-prompting ? > > I did not see a compile error (I didn't enable it on any non-ARM > platform). > > But it is rather futile to ask the user about (thousands of) drivers > for hardware that cannot possibly be present on the system he is > configuring a kernel for. Understood, as of now it is certainly high-unlikely to find such an ARM IP on a non-ARM SoC and I suppose that we can anyway drop this if ever the day will come that such a system will appear. Fine for me. Thanks, Cristian
On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > Hi Cristian, > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > <cristian.marussi@arm.com> wrote: > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > dependency on ARM64, to prevent asking the user about this driver when > > > configuring a kernel for a different architecture than ARM64. > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > this series since, despite this being an ARM IP, it has really no technical > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > platform driver, so it seemed an uneeded artificial restriction to impose... > > ...having said that, surely my live testing were performed only on arm64 models > > as of now. > > For that, we have COMPILE_TEST=y. > > > So, I am not saying that I am against this proposed fix but what is the > > issue that is trying to solve, have you seen any compilation error ? or > > is it just to avoid the user-prompting ? > > I did not see a compile error (I didn't enable it on any non-ARM > platform). > > But it is rather futile to ask the user about (thousands of) drivers > for hardware that cannot possibly be present on the system he is > configuring a kernel for. I am fine with this fix but I have seen quite opposite argument. That is not to add dependency if it is not strictly required. Also since you state that the fix is to avoid users of other archs being posed with the question that they may get annoyed or can't answer, I wonder if the right approach is to make this driver default "n" instead. I don't know what is the right or preferred approach here. I am fine either way. -- Regards, Sudeep
Hi Sudeep, On Mon, Jun 3, 2024 at 3:39 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > > <cristian.marussi@arm.com> wrote: > > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > > dependency on ARM64, to prevent asking the user about this driver when > > > > configuring a kernel for a different architecture than ARM64. > > > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > > this series since, despite this being an ARM IP, it has really no technical > > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > > platform driver, so it seemed an uneeded artificial restriction to impose... > > > ...having said that, surely my live testing were performed only on arm64 models > > > as of now. > > > > For that, we have COMPILE_TEST=y. > > > > > So, I am not saying that I am against this proposed fix but what is the > > > issue that is trying to solve, have you seen any compilation error ? or > > > is it just to avoid the user-prompting ? > > > > I did not see a compile error (I didn't enable it on any non-ARM > > platform). > > > > But it is rather futile to ask the user about (thousands of) drivers > > for hardware that cannot possibly be present on the system he is > > configuring a kernel for. > > I am fine with this fix but I have seen quite opposite argument. That is > not to add dependency if it is not strictly required. Can you please point me to that reference? > Also since you state that the fix is to avoid users of other archs being > posed with the question that they may get annoyed or can't answer, I > wonder if the right approach is to make this driver default "n" instead. The driver already defaults to "n" (which is the default default ;-) Thanks! Gr{oetje,eeting}s, Geert
On Mon, Jun 03, 2024 at 07:52:56PM +0200, Geert Uytterhoeven wrote: > Hi Sudeep, > > On Mon, Jun 3, 2024 at 3:39 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > > > <cristian.marussi@arm.com> wrote: > > > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > > > dependency on ARM64, to prevent asking the user about this driver when > > > > > configuring a kernel for a different architecture than ARM64. > > > > > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > > > this series since, despite this being an ARM IP, it has really no technical > > > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > > > platform driver, so it seemed an uneeded artificial restriction to impose... > > > > ...having said that, surely my live testing were performed only on arm64 models > > > > as of now. > > > > > > For that, we have COMPILE_TEST=y. > > > > > > > So, I am not saying that I am against this proposed fix but what is the > > > > issue that is trying to solve, have you seen any compilation error ? or > > > > is it just to avoid the user-prompting ? > > > > > > I did not see a compile error (I didn't enable it on any non-ARM > > > platform). > > > > > > But it is rather futile to ask the user about (thousands of) drivers > > > for hardware that cannot possibly be present on the system he is > > > configuring a kernel for. > > > > I am fine with this fix but I have seen quite opposite argument. That is > > not to add dependency if it is not strictly required. > > Can you please point me to that reference? > I don't have one handy, I need to dig but I have been asked to remove in the past. > > Also since you state that the fix is to avoid users of other archs being > > posed with the question that they may get annoyed or can't answer, I > > wonder if the right approach is to make this driver default "n" instead. > > The driver already defaults to "n" (which is the default default ;-) Ah Cristian mentioned the same in private. I may have misunderstood then, for some reason I thought explicit default "n" would avoid getting the prompt. As I said I am fine with the proposed change, just took this discussion as a way to learn little more about Kconfig. -- Regards, Sudeep
On Tue, Jun 04, 2024 at 08:07:18AM +0100, Sudeep Holla wrote: > On Mon, Jun 03, 2024 at 07:52:56PM +0200, Geert Uytterhoeven wrote: > > Hi Sudeep, > > > > On Mon, Jun 3, 2024 at 3:39 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > > > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > > > > <cristian.marussi@arm.com> wrote: > > > > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > > > > dependency on ARM64, to prevent asking the user about this driver when > > > > > > configuring a kernel for a different architecture than ARM64. > > > > > > > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > > > > this series since, despite this being an ARM IP, it has really no technical > > > > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > > > > platform driver, so it seemed an uneeded artificial restriction to impose... > > > > > ...having said that, surely my live testing were performed only on arm64 models > > > > > as of now. > > > > > > > > For that, we have COMPILE_TEST=y. > > > > > > > > > So, I am not saying that I am against this proposed fix but what is the > > > > > issue that is trying to solve, have you seen any compilation error ? or > > > > > is it just to avoid the user-prompting ? > > > > > > > > I did not see a compile error (I didn't enable it on any non-ARM > > > > platform). > > > > > > > > But it is rather futile to ask the user about (thousands of) drivers > > > > for hardware that cannot possibly be present on the system he is > > > > configuring a kernel for. > > > > > > I am fine with this fix but I have seen quite opposite argument. That is > > > not to add dependency if it is not strictly required. > > > > Can you please point me to that reference? > > > > I don't have one handy, I need to dig but I have been asked to remove > in the past. > > > > Also since you state that the fix is to avoid users of other archs being > > > posed with the question that they may get annoyed or can't answer, I > > > wonder if the right approach is to make this driver default "n" instead. > > > > The driver already defaults to "n" (which is the default default ;-) > > Ah Cristian mentioned the same in private. I may have misunderstood > then, for some reason I thought explicit default "n" would avoid getting > the prompt. > I just tried this trick, it does not seem to work: an explict default-n will anyway trigger a prompt. > As I said I am fine with the proposed change, just took this discussion > as a way to learn little more about Kconfig. > Can this be at least depends on ARM || ARM64 || COMPILE_TEST Thanks, Cristian
Hi Cristian, On Tue, Jun 4, 2024 at 11:46 AM Cristian Marussi <cristian.marussi@arm.com> wrote: > On Tue, Jun 04, 2024 at 08:07:18AM +0100, Sudeep Holla wrote: > > On Mon, Jun 03, 2024 at 07:52:56PM +0200, Geert Uytterhoeven wrote: > > > On Mon, Jun 3, 2024 at 3:39 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Wed, May 29, 2024 at 01:36:42PM +0200, Geert Uytterhoeven wrote: > > > > > On Wed, May 29, 2024 at 12:13 PM Cristian Marussi > > > > > <cristian.marussi@arm.com> wrote: > > > > > > On Wed, May 29, 2024 at 09:30:45AM +0200, Geert Uytterhoeven wrote: > > > > > > > The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a > > > > > > > dependency on ARM64, to prevent asking the user about this driver when > > > > > > > configuring a kernel for a different architecture than ARM64. > > > > > > > > > > > > the ARM64 dependency was dropped on purpose after a few iterations of > > > > > > this series since, despite this being an ARM IP, it has really no technical > > > > > > dependency on ARM arch, not even the usual one on ARM AMBA bus, being this a > > > > > > platform driver, so it seemed an uneeded artificial restriction to impose... > > > > > > ...having said that, surely my live testing were performed only on arm64 models > > > > > > as of now. > > > > > > > > > > For that, we have COMPILE_TEST=y. > > > > > > > > > > > So, I am not saying that I am against this proposed fix but what is the > > > > > > issue that is trying to solve, have you seen any compilation error ? or > > > > > > is it just to avoid the user-prompting ? > > > > > > > > > > I did not see a compile error (I didn't enable it on any non-ARM > > > > > platform). > > > > > > > > > > But it is rather futile to ask the user about (thousands of) drivers > > > > > for hardware that cannot possibly be present on the system he is > > > > > configuring a kernel for. > > > > > > > > I am fine with this fix but I have seen quite opposite argument. That is > > > > not to add dependency if it is not strictly required. > > > > > > Can you please point me to that reference? > > > > I don't have one handy, I need to dig but I have been asked to remove > > in the past. I guess Linus Torvalds has missed the "ARM MHUv3 Mailbox" question when configuring his kernel.... Or he has disabled CONFIG_MAILBOX (it is not enabled in any but a few arm defconfigs). Oh wait, he runs ARM64 now, so the question was valid ;-) > > > > Also since you state that the fix is to avoid users of other archs being > > > > posed with the question that they may get annoyed or can't answer, I > > > > wonder if the right approach is to make this driver default "n" instead. > > > > > > The driver already defaults to "n" (which is the default default ;-) > > > > Ah Cristian mentioned the same in private. I may have misunderstood > > then, for some reason I thought explicit default "n" would avoid getting > > the prompt. > > I just tried this trick, it does not seem to work: an explict default-n will > anyway trigger a prompt. The default value does not control the visibility. Visibility can only be controlled through "{bool,tristate} ... if <condition>", or through "depends on <condition>". > > As I said I am fine with the proposed change, just took this discussion > > as a way to learn little more about Kconfig. > > Can this be at least > > depends on ARM || ARM64 || COMPILE_TEST Only if the MHUv3 can also be present on ARM ("aarch32") SoCs. Or do people really run 32-bit kernels on ARM64? Gr{oetje,eeting}s, Geert
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index 3b8842c4a3401579..8d4d1cbb1d4ca625 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -25,6 +25,7 @@ config ARM_MHU_V2 config ARM_MHU_V3 tristate "ARM MHUv3 Mailbox" + depends on ARM64 || COMPILE_TEST depends on HAS_IOMEM || COMPILE_TEST depends on OF help
The ARM MHUv3 controller is only present on ARM64 SoCs. Hence add a dependency on ARM64, to prevent asking the user about this driver when configuring a kernel for a different architecture than ARM64. Fixes: ca1a8680b134b5e6 ("mailbox: arm_mhuv3: Add driver") Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- The MHUv3 documentation is unclear about whether this can be present only on ARM64, or also on ARM. Googling 'site:arm.com MHUv3 "aarch32"' shows no results. Googling 'site:arm.com MHUv3 "aarch64"' does show results. --- drivers/mailbox/Kconfig | 1 + 1 file changed, 1 insertion(+)