Input: tm2-touchkey - add hardware dependency
Subsystems (2)
RFC PATCH 1/1 Input: tm2-touchkey - add hardware dependency
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
---
Note: I am not familar with the hardware in question so I'm not sure if this is the right dependency to add, or if ARM64 would be more appropriate, or something else, or a combination thereof. Please advise.
drivers/input/keyboard/Kconfig | 1 + 1 file changed, 1 insertion(+)
drivers/input/keyboard/Kconfig +1 -0
@@ -670,6 +670,7 @@ config KEYBOARD_TM2_TOUCHKEY tristate "TM2 touchkey support" depends on I2C depends on LEDS_CLASS + depends on ARCH_EXYNOS || COMPILE_TEST help Say Y here to enable device driver for tm2-touchkey with LED control for the Exynos5433 TM2 board.
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 2 lines
Signed-off-by Jean Delvare <[email protected]>Fixes 72d1f2346ded ("Input: tm2-touchkey - add touchkey driver support for TM2")
I do not see a bug to fix. Even if we agree that driver building should be limited to Exynos then definitely there is no missing dependency to fix.
Best regards,
Krzysztof
Quoted 32 lines
Cc Jaechul Lee <[email protected]>Cc Beomho Seo <[email protected]>Cc Javier Martinez Canillas <[email protected]>Cc Andi Shyti <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Rob Herring <[email protected]>Cc Dmitry Torokhov <[email protected]>---
Note: I am not familar with the hardware in question so I'm not sure if this is the right dependency to add, or if ARM64 would be more appropriate, or something else, or a combination thereof. Please advise.drivers/input/keyboard/Kconfig | 1 + 1 file changed, 1 insertion(+)drivers/input/keyboard/Kconfig 2017-04-24 09:26:43.314252700 +0200 renamed from drivers/input/keyboard/Kconfig 2017-04-03 02:23:54.000000000 +0200 +1
-1@@ -670,6 +670,7 @@ config KEYBOARD_TM2_TOUCHKEY tristate "TM2 touchkey support" depends on I2C depends on LEDS_CLASS + depends on ARCH_EXYNOS || COMPILE_TEST help Say Y here to enable device driver for tm2-touchkey with LED control for the Exynos5433 TM2 board. --Jean Delvare
SUSE L3 Support
Hi Krzysztof,
Thanks for your quick answer.
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer. This is still expressed in terms of dependency in Kconfig terms (but a soft one, thus the || COMPILE_TEST.)
Quoted 6 lines
Quoted 2 lines
Signed-off-by Jean Delvare <[email protected]>Fixes 72d1f2346ded ("Input: tm2-touchkey - add touchkey driver support for TM2")I do not see a bug to fix. Even if we agree that driver building should be limited to Exynos then definitely there is no missing dependency to fix.
I can drop the Fixes tag, no problem.
Quoted 32 lines
Quoted 32 lines
Cc Jaechul Lee <[email protected]>Cc Beomho Seo <[email protected]>Cc Javier Martinez Canillas <[email protected]>Cc Andi Shyti <[email protected]>Cc Chanwoo Choi <[email protected]>Cc Krzysztof Kozlowski <[email protected]>Cc Rob Herring <[email protected]>Cc Dmitry Torokhov <[email protected]>---
Note: I am not familar with the hardware in question so I'm not sure if this is the right dependency to add, or if ARM64 would be more appropriate, or something else, or a combination thereof. Please advise.drivers/input/keyboard/Kconfig | 1 + 1 file changed, 1 insertion(+)drivers/input/keyboard/Kconfig 2017-04-24 09:26:43.314252700 +0200 renamed from drivers/input/keyboard/Kconfig 2017-04-03 02:23:54.000000000 +0200 +1
-1@@ -670,6 +670,7 @@ config KEYBOARD_TM2_TOUCHKEY tristate "TM2 touchkey support" depends on I2C depends on LEDS_CLASS + depends on ARCH_EXYNOS || COMPILE_TEST help Say Y here to enable device driver for tm2-touchkey with LED control for the Exynos5433 TM2 board. --Jean Delvare
SUSE L3 Support
Quoted 10 lines · Jean Delvare
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 7 lines
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer.
You need a depends-like version of "imply" keyword. I think it is worth adding it to solve such problems and help in configuring the system. However I am not convinced that "depends" should be used in the meaning of "intended use".
Best regards,
Krzysztof
Quoted 19 lines · Krzysztof Kozlowski
Quoted 10 lines · Jean Delvare
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 7 lines
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer.
Quoted 4 lines
You need a depends-like version of "imply" keyword. I think it is worth adding it to solve such problems and help in configuring the system. However I am not convinced that "depends" should be used in the meaning of "intended use".
You are a bit late to the party I am afraid. COMPILE_TEST was introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.
To be honest, I have also considered the possibility of a dedicated keyword to express these "intended hardware target" soft dependencies. Maybe it would make things clearer. But I never had the time to look into it. Feel free to propose something if you are interested.
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 25 lines · Jean Delvare
Quoted 19 lines · Krzysztof Kozlowski
Quoted 10 lines · Jean Delvare
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 7 lines
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer.
Quoted 4 lines
You need a depends-like version of "imply" keyword. I think it is worth adding it to solve such problems and help in configuring the system. However I am not convinced that "depends" should be used in the meaning of "intended use".
Quoted 4 lines
You are a bit late to the party I am afraid.
COMPILE_TESTwas introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.
That is not the purpose of COMPILE_TEST. It serves only to allow compile testing of everything, not selecting "soft dependencies". It allows you to build kernel which will not even work in certain cases.
Also it does not bring any information about wanted or unwanted links
- like "imply".
Quoted 4 lines
To be honest, I have also considered the possibility of a dedicated keyword to express these "intended hardware target" soft dependencies. Maybe it would make things clearer. But I never had the time to look into it. Feel free to propose something if you are interested.
Workaround might be using default = N, unless ARCH_EXYNOS. Something like:
default y if ARCH_EXYNOS
Best regards,
Krzysztof
Dmitry Torokhov
maintainerQuoted 38 lines · Krzysztof Kozlowski
Quoted 25 lines · Jean Delvare
Quoted 19 lines · Krzysztof Kozlowski
Quoted 10 lines · Jean Delvare
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 7 lines
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer.
Quoted 4 lines
You need a depends-like version of "imply" keyword. I think it is worth adding it to solve such problems and help in configuring the system. However I am not convinced that "depends" should be used in the meaning of "intended use".
Quoted 4 lines
You are a bit late to the party I am afraid.
COMPILE_TESTwas introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.That is not the purpose of
COMPILE_TEST. It serves only to allow compile testing of everything, not selecting "soft dependencies". It allows you to build kernel which will not even work in certain cases.Also it does not bring any information about wanted or unwanted links
- like "imply".
Quoted 7 lines
Quoted 4 lines
To be honest, I have also considered the possibility of a dedicated keyword to express these "intended hardware target" soft dependencies. Maybe it would make things clearer. But I never had the time to look into it. Feel free to propose something if you are interested.
Workaround might be using default = N, unless
ARCH_EXYNOS. Something like:
default y ifARCH_EXYNOS
This would strongly imply that the device is available on all Exynos boards. I am fairly certain that our Chromebooks, for example, do not have it.
Besides, I do not see what is wrong with driver being offered on a platform that does not have it. This happens all the time. Consider I2C stuff:
config I2C_I801
tristate "Intel 82801 (ICH/PCH)" depends on PCI select CHECK_SIGNATURE if X86 && DMI select I2C_SMBUS
This is an X86 device, but is offered everywhere where we have PCI.
Thanks.
Signature · 1 line
Dmitry
Quoted 7 lines · Dmitry Torokhov
Quoted 2 lines · Krzysztof Kozlowski
Workaround might be using default = N, unless
ARCH_EXYNOS. Something like:
default y ifARCH_EXYNOSThis would strongly imply that the device is available on all Exynos boards. I am fairly certain that our Chromebooks, for example, do not have it.
A new Kconfig keyword like "usable on" would imply that as well... I do not know how you want to limit this to only few specific boards.
Quoted 5 lines
Besides, I do not see what is wrong with driver being offered on a platform that does not have it. This happens all the time. Consider I2C stuff:
Quoted 7 lines
config
I2C_I801tristate "Intel 82801 (ICH/PCH)" depends on PCI select CHECK_SIGNATURE if X86 && DMI select I2C_SMBUSThis is an X86 device, but is offered everywhere where we have PCI.
But that I2C_I801 driver has a real dependency on PCI. It just needs the PCI. This is why you have depends for.
Other drivers who exist only on Exynos boards do not have such dependencies. They have only real hardware or software "depends", e.g.:
config MFD_MAX77686
tristate "Maxim Semiconductor MAX77686/802 PMIC Support" depends on I2C
depends on OF || COMPILE_TEST
select MFD_CORE
select REGMAP_I2C
select REGMAP_IRQ
select IRQ_DOMAIN
This driver needs I2C and OF. It does not need Exynos for anything, although it is used only for Exynos specific boards so far.
Modeling drivers that "exist only on two Exynos based boards" through depending on ARCH_EXYNOS is different. Not mentioning that the TM2 touch key is in fact Cypress touchkey and could be used on other boards.
Best regards,
Krzysztof
Hi Dmitry,
Quoted 14 lines · Dmitry Torokhov
Quoted 6 lines · Krzysztof Kozlowski
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines · Krzysztof Kozlowski
Quoted 5 lines · Jean Delvare
Quoted 3 lines
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Quoted 6 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
(...)
Workaround might be using default = N, unless
ARCH_EXYNOS. Something like:
default y ifARCH_EXYNOS
Quoted 6 lines
This would strongly imply that the device is available on all Exynos boards. I am fairly certain that our Chromebooks, for example, do not have it.
Besides, I do not see what is wrong with driver being offered on a platform that does not have it.
It makes the life of distribution kernel maintainers a lot harder, by asking them questions that are irrelevant. They may eventually figure it out and say "no", but it costs them time to investigate each option. Worst case they will accidentally say yes and bloat their kernel.
Quoted 1 line
This happens all the time.
That doesn't make it right. The trend is changing, and you should embrace that change. What was acceptable 10 days ago when we had 7000 kernel options no longer works today with 17000.
Quoted 1 line
Consider I2C stuff:
Trying to make it personal? ;-)
Quoted 7 lines
config
I2C_I801tristate "Intel 82801 (ICH/PCH)" depends on PCI select CHECK_SIGNATURE if X86 && DMI select I2C_SMBUSThis is an X86 device, but is offered everywhere where we have PCI.
In fact the 82801 family of south bridges is not limited to X86, it can be found on IA64 hardware as well. It was discussed here:
https://patchwork.ozlabs.org/patch/635865/
and as you can see I would have liked the hardware dependency to be added, it's Andy Shevchenko who declined :-(
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 26 lines · Krzysztof Kozlowski
Quoted 25 lines · Jean Delvare
Quoted 23 lines · Krzysztof Kozlowski
Quoted 10 lines · Jean Delvare
Quoted 8 lines · Krzysztof Kozlowski
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
Although the device exists in only two upstreamed Exynos boards but there is no hardware dependency on Exynos. The hardware does not depend on Exynos.
Quoted 7 lines
I understand that, and this is the reason why there was no dependency expressed so far. But this is irrelevant to the problem I am trying to solve, which is that people configuring a kernel for platforms where this device is known to NOT exist shouldn't be bothered with a question about its driver. This is what I meant with "hardware dependency" but you can call it "hardware focus" or "intended hardware target" if you prefer.
You need a depends-like version of "imply" keyword. I think it is worth adding it to solve such problems and help in configuring the system. However I am not convinced that "depends" should be used in the meaning of "intended use".
Quoted 7 lines
Quoted 4 lines
You are a bit late to the party I am afraid.
COMPILE_TESTwas introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.That is not the purpose of
COMPILE_TEST. It serves only to allow compile testing of everything, not selecting "soft dependencies".
Think again and you'll realize this is exactly the same.
Quoted 2 lines
It
allows you to build kernel which will not even work in certain cases.
This is an extreme theoretical case, which has nothing to do with the problem at hand.
Quoted 2 lines
Also it does not bring any information about wanted or unwanted links
- like "imply".
I have no idea what you mean here, sorry.
Quoted 7 lines
Quoted 4 lines
To be honest, I have also considered the possibility of a dedicated keyword to express these "intended hardware target" soft dependencies. Maybe it would make things clearer. But I never had the time to look into it. Feel free to propose something if you are interested.
Workaround might be using default = N, unless
ARCH_EXYNOS. Something like:
default y ifARCH_EXYNOS
Maybe this is a good idea, maybe not, I don't know as I am not familiar with this platform nor the ARM world in general. What I am sure of is that this has nothing to do with the problem I was originally discussing. I want to change the visibility on non-Exynos build, you propose to change the default on Exynos builds. That's not helping (not me, at least.)
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 10 lines · Jean Delvare
Quoted 9 lines · Krzysztof Kozlowski
Quoted 7 lines · Jean Delvare
Quoted 2 lines · Krzysztof Kozlowski
Quoted 1 line · Jean Delvare
On Mon, 24 Apr 2017 10:00:32 +0200, Krzysztof Kozlowski wrote:
You are a bit late to the party I am afraid.
COMPILE_TESTwas introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.
Quoted 4 lines
Quoted 2 lines
That is not the purpose of
COMPILE_TEST. It serves only to allow compile testing of everything, not selecting "soft dependencies".Think again and you'll realize this is exactly the same.
No, the purpose of COMPILE_TEST is to build everything whenever it is possible. Regardless if it is reasonable or not. For example without OF some drivers will not be able to bind but we can build them for build coverage (and static checkers coverage). That is the goal.
However you want to limit the driver because of specific board existing in mainline (have you considered out of tree boards?). That is totally different meaning and use case.
Quoted 5 lines
Quoted 2 lines
It
allows you to build kernel which will not even work in certain cases.This is an extreme theoretical case, which has nothing to do with the problem at hand.
That is the effect of COMPILE_TEST for which it was designed... but you want to use it in different way.
Quoted 5 lines
Quoted 3 lines
Also it does not bring any information about wanted or unwanted links
- like "imply".I have no idea what you mean here, sorry.
The depends/select/imply bring an information to the user about relationship between the Kconfig symbols.
COMPILE_TEST does not bring such relationship.
Quoted 9 lines
Quoted 8 lines
Quoted 5 lines
To be honest, I have also considered the possibility of a dedicated keyword to express these "intended hardware target" soft dependencies. Maybe it would make things clearer. But I never had the time to look into it. Feel free to propose something if you are interested.
Workaround might be using default = N, unless
ARCH_EXYNOS. Something like:
default y ifARCH_EXYNOS
Quoted 6 lines
Maybe this is a good idea, maybe not, I don't know as I am not familiar with this platform nor the ARM world in general. What I am sure of is that this has nothing to do with the problem I was originally discussing. I want to change the visibility on non-Exynos build, you propose to change the default on Exynos builds. That's not helping (not me, at least.)
First of all, I am not changing the defaults. Second, you asked for a hint of a better solution and now you are complaining that you do not like it... okay, as you wish.
The other way is to introduce a new soft dependency called whatever you like to (e.g. "usable on").
But depends and COMPILE_TEST are not for that purpose.
Best regards,
Krzysztof
Hi Krzysztof,
I'll try to stay calm but no promise :-D
Quoted 13 lines · Krzysztof Kozlowski
Quoted 12 lines · Jean Delvare
Quoted 10 lines · Krzysztof Kozlowski
Quoted 6 lines · Jean Delvare
Quoted 1 line · Krzysztof Kozlowski
On Mon, Apr 24, 2017 at 11:48 AM, Jean Delvare <[email protected]> wrote:
You are a bit late to the party I am afraid.
COMPILE_TESTwas introduced for this very usage 4 years ago and I count 760 occurrences of it. Not as many as I would like but I think this is going in the right direction.That is not the purpose of
COMPILE_TEST. It serves only to allow compile testing of everything, not selecting "soft dependencies".
Quoted 6 lines
Quoted 1 line
Think again and you'll realize this is exactly the same.
No, the purpose of
COMPILE_TESTis to build everything whenever it is possible. Regardless if it is reasonable or not. For example without OF some drivers will not be able to bind but we can build them for build coverage (and static checkers coverage). That is the goal.
I will concede that the short description of COMPILE_TEST ("Compile also drivers which will not load") can be misleading. It's not only about "not loading." But if you take a look at the help text, it is more verbose and accurate:
"Some drivers can be compiled on a different platform than they are intended to be run on. Despite they cannot be loaded there (or even when they load they cannot be used due to missing HW support), developers still, opposing to distributors, might want to build such drivers to compile-test them.
If you are a developer and want to build everything available, say Y here. If you are a user/distributor, say N here to exclude useless drivers to be distributed."
I give you a few minutes to read the whole thing.
Please pay attention to "due to missing HW support", "opposing to distributors" and "say N here to exclude useless drivers". So, to summarize, the purpose of COMPILE_TEST is to help distribution kernel maintainers exclude useless drivers while still allowing a maximum build testing coverage of such drivers.
What I am trying to do here is to help distribution kernel maintainers exclude the tm2 touchkey driver from kernel flavors where it would be useless (this is "depends on ARCH_EXYNOS" in my patch) while not hurting build test coverage (this is "|| COMPILE_TEST" in my patch.) Therefore I am clearly using COMPILE_TEST for its intended purpose. QED.
Quoted 3 lines
However you want to limit the driver because of specific board existing in mainline (have you considered out of tree boards?). That is totally different meaning and use case.
I never mentioned specific boards. I proposed ARCH_EXYNOS, which to the best of my knowledge is a sub-architecture or platform if you will. Additionally, if you read my original post again, you will notice that I explicitly said that I was not certain if it was the most appropriate "level" of visibility for this driver. What seems clear to me is that proposing this driver when I build an X86 kernel does not make sense.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
(...) It
allows you to build kernel which will not even work in certain cases.This is an extreme theoretical case, which has nothing to do with the problem at hand.
That is the effect of
COMPILE_TESTfor which it was designed... but you want to use it in different way.
No. I invite you to read the help text of COMPILE_TEST again. It was not designed to build kernels which will not work. Nobody needed that.
Quoted 5 lines
(...)
The depends/select/imply bring an information to the user about relationship between theKconfigsymbols.
COMPILE_TESTdoes not bring such relationship.
I thank you for trying to clarify what you meant, but even after reading it 3 times, I still have no idea what you mean. Honestly.
Quoted 5 lines
Quoted 3 lines
Quoted 3 lines
(...)
Workaround might be using default = N, unlessARCH_EXYNOS. Something like:
default y ifARCH_EXYNOSFirst of all, I am not changing the defaults.
Oh well, what can I say. I think I'll let you keep arguing with yourself. This will save me a lot of time to devote to other tasks with higher chances of success.
Quoted 3 lines
Second, you asked for a
hint of a better solution and now you are complaining that you do not like it... okay, as you wish.
I asked for suggestions on a specific point. Let me quote myself:
"I'm not sure if this is the right dependency to add, or if ARM64 would be more appropriate, or something else (...) Please advise."
I did not ask for a lesson about what COMPILE_TEST is meant for, nor for suggestions of what the default value should be. I asked for advice on what the visibility should be.
Quoted 2 lines
The other way is to introduce a new soft dependency called whatever you like to (e.g. "usable on").
Again, this may be a good idea that can be discussed, but that keyword does not exist today, and regular depends + COMPILE_TEST have been used for the purpose since 2013. Feel free to start a separate discussion thread about introducing "usable on" as a replacement for COMPILE_TEST, maybe with a proof of concept, and see what people think about your idea.
Quoted 1 line
But depends and
COMPILE_TESTare not for that purpose.
... *sigh*.
This is my last reply to you on this topic, as we are clearly running in circles.
Signature · 2 lines
Jean Delvare SUSE L3 Support
Hi Jean,
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
I'm sorry, I don't see any connection between Exynos and the TM2-touchkey (which is manufactured by cypress, btw).
It surely does have connections with the TM2 board which happens to have an Exynos SoC, but that cannot be fixed from the Kconfig.
Thanks,
Andi
Hi Andi,
Quoted 10 lines · Andi Shyti
Quoted 3 lines · Jean Delvare
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
I'm sorry, I don't see any connection between Exynos and the TM2-touchkey (which is manufactured by cypress, btw).
It surely does have connections with the TM2 board which happens to have an Exynos SoC, but that cannot be fixed from the
Kconfig.
Thanks for your constructive contribution to this discussion.
The KEYBOARD_TM2_TOUCHKEY option name, description and help text as well as the driver name all make it sound highly hardware specific. If this driver is for a "generic" part from Cypress then the driver should be renamed and the Kconfig option name and description should be updated to reflect that. The help text should also first mention the chip vendor and name, and only then mention that it is used on the Exynos5433 TM2 board.
That being said, I took a look at the driver, and while "cypress" is indeed being mentioned as the vendor name, the chip name itself is referred to as "tm2-touchkey", which isn't generic at all. You can't identify the hardware under the name "tm2 touchkey" everywhere in the driver and expect people to understand it is not board specific.
So it really boils down to this question: is that chip a generic part from Cypress, and if so, what is the real part number? Or was is designed privately by Cypress specifically for Samsung for this one board (and possibly others to come)?
Thanks again,
Signature · 2 lines
Jean Delvare SUSE L3 Support
Hi Jean,
Quoted 27 lines
Quoted 10 lines · Jean Delvare
Quoted 3 lines
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
I'm sorry, I don't see any connection between Exynos and the TM2-touchkey (which is manufactured by cypress, btw).
It surely does have connections with the TM2 board which happens to have an Exynos SoC, but that cannot be fixed from the
Kconfig.Thanks for your constructive contribution to this discussion.
The
KEYBOARD_TM2_TOUCHKEYoption name, description and help text as well as the driver name all make it sound highly hardware specific. If this driver is for a "generic" part from Cypress then the driver should be renamed and theKconfigoption name and description should be updated to reflect that. The help text should also first mention the chip vendor and name, and only then mention that it is used on the Exynos5433 TM2 board.That being said, I took a look at the driver, and while "cypress" is indeed being mentioned as the vendor name, the chip name itself is referred to as "tm2-touchkey", which isn't generic at all. You can't identify the hardware under the name "tm2 touchkey" everywhere in the driver and expect people to understand it is not board specific.
Quoted 4 lines
So it really boils down to this question: is that chip a generic part from Cypress, and if so, what is the real part number? Or was is designed privately by Cypress specifically for Samsung for this one board (and possibly others to come)?
I knew that the naming was bringing confusion and we had a previous discussion about it with Chanwoo [1].
This is indeed a generic device from Cypress. The driver has been ported from Android's Kernel [2]; it says that the device part is cy8cmbr3xxx, but the datasheet [3] doesn't have any connection with what the TM2 board has (i.e. the registers don't match). That's why we suspected that (as you said) this might be a touch key sensor specifically designed for the TM2 board.
Cypress was not that helpful.
The alternative was to not provide support, but it didn't look right.
Thanks,
Andi
[1] https://marc.info/?l=linux-kernel&m=148374097826303&w=2
[2] http://opensource.samsung.com/reception/receptionSub.do?method=sub&sub=F&searchValue=sm-n910K
[3] http://www.cypress.com/documentation/datasheets/cy8cmbr3002-cy8cmbr3102-cy8cmbr3106s-cy8cmbr3108-cy8cmbr3110-cy8cmbr3116
Dmitry Torokhov
maintainerQuoted 44 lines · Andi Shyti
Hi Jean,
Quoted 27 lines
Quoted 10 lines · Jean Delvare
Quoted 3 lines
The tm2-touchkey driver is only useful on specific platforms. Add the missing hardware dependency so that the driver is not proposed on systems where the device does not exist.
I'm sorry, I don't see any connection between Exynos and the TM2-touchkey (which is manufactured by cypress, btw).
It surely does have connections with the TM2 board which happens to have an Exynos SoC, but that cannot be fixed from the
Kconfig.Thanks for your constructive contribution to this discussion.
The
KEYBOARD_TM2_TOUCHKEYoption name, description and help text as well as the driver name all make it sound highly hardware specific. If this driver is for a "generic" part from Cypress then the driver should be renamed and theKconfigoption name and description should be updated to reflect that. The help text should also first mention the chip vendor and name, and only then mention that it is used on the Exynos5433 TM2 board.That being said, I took a look at the driver, and while "cypress" is indeed being mentioned as the vendor name, the chip name itself is referred to as "tm2-touchkey", which isn't generic at all. You can't identify the hardware under the name "tm2 touchkey" everywhere in the driver and expect people to understand it is not board specific.
Quoted 4 lines
So it really boils down to this question: is that chip a generic part from Cypress, and if so, what is the real part number? Or was is designed privately by Cypress specifically for Samsung for this one board (and possibly others to come)?
I knew that the naming was bringing confusion and we had a previous discussion about it with Chanwoo [1].
This is indeed a generic device from Cypress. The driver has been ported from Android's Kernel [2]; it says that the device part is cy8cmbr3xxx, but the datasheet [3] doesn't have any connection with what the TM2 board has (i.e. the registers don't match). That's why we suspected that (as you said) this might be a touch key sensor specifically designed for the TM2 board.
Quoted 4 lines
Cypress was not that helpful.
The alternative was to not provide support, but it didn't look right.
So this looks like we are dealing with a device designed for a specific board, but not architecture or board-specific. Similar to atmel_captouch, ims_pcu, or all drivers living in platform/x86 (I understand that they do not bother Jean simply because he cares mostly about x86 and, with SUSE being generic distro, he needs to enable all the drivers there anyway, but a person configuring "their" kernel still has to go and consider all options).
I am all for adding dependencies for drivers that are parts of particular SoCs (you probably not going to have Broardcom IPROC touchscreen on your x86 device), but external to SoC peripherals is a different story.
Thanks.
Signature · 1 line
Dmitry
Hi Dmitry,
Quoted 7 lines · Dmitry Torokhov
So this looks like we are dealing with a device designed for a specific board, but not architecture or board-specific. Similar to
atmel_captouch,ims_pcu, or all drivers living in platform/x86 (I understand that they do not bother Jean simply because he cares mostly about x86 and, with SUSE being generic distro, he needs to enable all the drivers there anyway, but a person configuring "their" kernel still has to go and consider all options).
Actually they do bother me at times (some of them can't be built as modules.) Also while I indeed care mostly about x86, openSUSE supports various other architectures, so I would be equally worried about x86-specific drivers being proposed on these architectures, for example. The only difference is that I typically do not catch these myself.
Quoted 4 lines
I am all for adding dependencies for drivers that are parts of particular SoCs (you probably not going to have Broardcom IPROC touchscreen on your x86 device), but external to SoC peripherals is a different story.
I understand that there is no direct relation between this TM2 touchkey driver and Exynos as a platform.
My reasoning is as follows: given that at this point this driver is only useful on the Samsung TM2 board, it should only be presented to users configuring a kernel for that board. Then the question becomes:
what other kernel option will definitely be enabled on any kernel running on that board? As the TM2 is based on an Exynos5433 SoC, ARCH_EXYNOS will have to be enabled, so it seems a convenient way to limit the visibility of KEYBOARD_TM2_TOUCHKEY.
There are certainly other options that will also be always enabled for a TM2 board kernel, but ARCH_EXYNOS seems to be a good choice because it is both generic enough that it doesn't need to be changed if a variation of the TM2 board is released using a slightly different SoC, and specific enough that we will skip the question for most users.
The alternative would be to add another option SAMSUNG_TM2 ("Support for the Samsung TM2 board"), just to let KEYBOARD_TM2_TOUCHKEY depend on it, but that's really only moving the problem, as there is no point in asking people about SAMSUNG_TM2 if they are building a kernel that can't support it anyway. And while I agree it would be somewhat cleaner to have KEYBOARD_TM2_TOUCHKEY which depends on SAMSUNG_TM2 and SAMSUNG_TM2 which depends on ARCH_EXYNOS, it also seems overly complex to me for no benefit.
Therefore I still believe that making KEYBOARD_TM2_TOUCHKEY depend on ARCH_EXYNOS || COMPILE_TEST makes sense from a practical perspective.
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 31 lines · Jean Delvare
Quoted 4 lines
I am all for adding dependencies for drivers that are parts of particular SoCs (you probably not going to have Broardcom IPROC touchscreen on your x86 device), but external to SoC peripherals is a different story.
I understand that there is no direct relation between this TM2 touchkey driver and Exynos as a platform.
My reasoning is as follows: given that at this point this driver is only useful on the Samsung TM2 board, it should only be presented to users configuring a kernel for that board. Then the question becomes:
what other kernel option will definitely be enabled on any kernel running on that board? As the TM2 is based on an Exynos5433 SoC,ARCH_EXYNOSwill have to be enabled, so it seems a convenient way to limit the visibility ofKEYBOARD_TM2_TOUCHKEY.There are certainly other options that will also be always enabled for a TM2 board kernel, but
ARCH_EXYNOSseems to be a good choice because it is both generic enough that it doesn't need to be changed if a variation of the TM2 board is released using a slightly different SoC, and specific enough that we will skip the question for most users.The alternative would be to add another option
SAMSUNG_TM2("Support for the Samsung TM2 board"), just to letKEYBOARD_TM2_TOUCHKEYdepend on it, but that's really only moving the problem, as there is no point in asking people aboutSAMSUNG_TM2if they are building a kernel that can't support it anyway. And while I agree it would be somewhat cleaner to haveKEYBOARD_TM2_TOUCHKEYwhich depends onSAMSUNG_TM2andSAMSUNG_TM2which depends onARCH_EXYNOS, it also seems overly complex to me for no benefit.
Quoted 2 lines
Therefore I still believe that making
KEYBOARD_TM2_TOUCHKEYdepend onARCH_EXYNOS||COMPILE_TESTmakes sense from a practical perspective.
As I mentioned at beginning, I limiting unusable options looks worth adding to the kernel as a new keyword. However dependency is not for that purpose because this is not a hardware/software dependency. You need to add something similar to "imply" keyword, but working not for select statements but for dependencies. I think such new keyword would be useful not only for Suse kernels but for many others as well.
But if you do not wish to do it right, then please do not go with this workaround making a hard (static) dependency between some driver and some ARM architecture.
On the other hand, maybe this particular driver problem for you can be solved by removing i2c device binding and adding dependency on OF. Assuming of course, you do not choose OF for your kernels.
Best regards,
Krzysztof
Hi Andi,
Quoted 9 lines · Andi Shyti
Quoted 5 lines
Quoted 1 line · Jean Delvare
So it really boils down to this question: is that chip a generic part from Cypress, and if so, what is the real part number? Or was is designed privately by Cypress specifically for Samsung for this one board (and possibly others to come)?
I knew that the naming was bringing confusion and we had a previous discussion about it with Chanwoo [1].
Quoted 6 lines
This is indeed a generic device from Cypress. The driver has been ported from Android's Kernel [2]; it says that the device part is cy8cmbr3xxx, but the datasheet [3] doesn't have any connection with what the TM2 board has (i.e. the registers don't match). That's why we suspected that (as you said) this might be a touch key sensor specifically designed for the TM2 board.
Thanks for the pointers, it helps.
Quoted 1 line
Cypress was not that helpful.
I've been there before with other manufacturers. Chipsets designed specifically for one hardware vendor are the hardest to support for this reason.
Quoted 2 lines
The alternative was to not provide support, but it didn't look right.
I agree, you did the right thing by getting the driver upstream. But it doesn't mean this driver must be enabled in all distribution kernels.
Signature · 2 lines
Jean Delvare SUSE L3 Support