Hi Greg,, I need a few clarifications before sending (hopefully) final revisions to the patch.
On 2021-01-31 11:45 p.m., Greg Kroah-Hartman wrote: > On Sun, Jan 31, 2021 at 03:30:49PM -0800, Scott Branden wrote: >> Correct compile issue if CONFIG_TTY is not set by >> only adding ttyVK devices if CONFIG_BCM_VK_TTY is set. >> >> Reported-by: Randy Dunlap <rdun...@infradead.org> >> Signed-off-by: Scott Branden <scott.bran...@broadcom.com> >> >> --- >> Changes since v2: >> - add CONFIG_BCM_VK_TTY >> - add function and stub for bcm_vk_tty_set_irq_enabled >> Changes since v1: >> - add function stubs rather than compiling out code >> --- >> drivers/misc/bcm-vk/Kconfig | 16 ++++++++++++ >> drivers/misc/bcm-vk/Makefile | 4 +-- >> drivers/misc/bcm-vk/bcm_vk.h | 42 +++++++++++++++++++++++++++++--- >> drivers/misc/bcm-vk/bcm_vk_dev.c | 5 ++-- >> drivers/misc/bcm-vk/bcm_vk_tty.c | 6 +++++ >> 5 files changed, 65 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/misc/bcm-vk/Kconfig b/drivers/misc/bcm-vk/Kconfig >> index 052f6f28b540..16ce98c964b8 100644 >> --- a/drivers/misc/bcm-vk/Kconfig >> +++ b/drivers/misc/bcm-vk/Kconfig >> @@ -15,3 +15,19 @@ config BCM_VK >> accelerators via /dev/bcm-vk.N devices. >> >> If unsure, say N. >> + >> +if BCM_VK > No need for this, just put it on the depends line, right? If you prefer I can but it on the depends on line. But, I actually prefer the if syntax in this case as it more clearly shows BCM_VK_TTY is a suboption of BCM_VK. Please let me know which method is "right"? > >> + >> +config BCM_VK_TTY >> + bool "Enable ttyVK" > Better config help text to explain what this is? I'll change it to the following? "Enable tty's on VK devices" > >> + depends on TTY >> + default y > Default y is only there if your system can not boot without it, please > remove it. I can remove if really needed but I'd like to learn more about such convention. Is there a document I can learn from describing such? We actually want a full featured driver by default. Otherwise we'll end up asking people to enable this feature and recompile the driver to get missing features such as this. > >> + help >> + Select this option to enable ttyVK support to allow console >> + access to VK cards from host. > Again, more help text, what is a "VK"? VK is already described in BCM_VK. Why would I need to add the same information again to a suboption? Perhaps you would like "config BCM_VK" changed to "menuconfig BCM_VK" > > thanks, > > greg k-h Thanks, Scott
