| Message ID | 70d91b0482e19d7551d3258ea54c970c1b996317.1627495116.git.ojaswin98@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: vchiq: Minor fixups to CONFIG_VCHIQ_CDEV | expand |
On Thu, Jul 29, 2021 at 12:07:16AM +0530, Ojaswin Mujoo wrote: > Before this config was defined, the cdev used to be created > unconditionally. When an earlier commit introduced this config, the > default behavior was set to disabled, which might surprise some > unsuspecting users. Hence, make this config default to 'Y' to be more > backward consistent. > > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com> > --- > drivers/staging/vc04_services/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig > index 63caa6818d37..2b70c37cdd09 100644 > --- a/drivers/staging/vc04_services/Kconfig > +++ b/drivers/staging/vc04_services/Kconfig > @@ -23,6 +23,7 @@ if BCM2835_VCHIQ > > config VCHIQ_CDEV > bool "VCHIQ Character Driver" > + default y default y is only if the machine will not work without this option. Is that the case here? If not, then please do not have this as the default. thanks, greg k-h
On Wed, Jul 28, 2021 at 08:45:55PM +0200, Greg KH wrote: > On Thu, Jul 29, 2021 at 12:07:16AM +0530, Ojaswin Mujoo wrote: > > Before this config was defined, the cdev used to be created > > unconditionally. When an earlier commit introduced this config, the > > default behavior was set to disabled, which might surprise some > > unsuspecting users. Hence, make this config default to 'Y' to be more > > backward consistent. > > > > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com> > > --- > > drivers/staging/vc04_services/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig > > index 63caa6818d37..2b70c37cdd09 100644 > > --- a/drivers/staging/vc04_services/Kconfig > > +++ b/drivers/staging/vc04_services/Kconfig > > @@ -23,6 +23,7 @@ if BCM2835_VCHIQ > > > > config VCHIQ_CDEV > > bool "VCHIQ Character Driver" > > + default y > > > default y is only if the machine will not work without this option. > Is that the case here? If not, then please do not have this as the > default. Got it Greg. From my testing, the Raspberry Pi does seem to boot correctly without this although some userspace libraries might not work. Since the machine itself works, I guess I'll drop this patch. Thank you! Ojaswin > > thanks, > > greg k-h
Am 28.07.21 um 22:06 schrieb Ojaswin Mujoo: > On Wed, Jul 28, 2021 at 08:45:55PM +0200, Greg KH wrote: >> On Thu, Jul 29, 2021 at 12:07:16AM +0530, Ojaswin Mujoo wrote: >>> Before this config was defined, the cdev used to be created >>> unconditionally. When an earlier commit introduced this config, the >>> default behavior was set to disabled, which might surprise some >>> unsuspecting users. Hence, make this config default to 'Y' to be more >>> backward consistent. >>> >>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com> >>> --- >>> drivers/staging/vc04_services/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig >>> index 63caa6818d37..2b70c37cdd09 100644 >>> --- a/drivers/staging/vc04_services/Kconfig >>> +++ b/drivers/staging/vc04_services/Kconfig >>> @@ -23,6 +23,7 @@ if BCM2835_VCHIQ >>> >>> config VCHIQ_CDEV >>> bool "VCHIQ Character Driver" >>> + default y >> >> default y is only if the machine will not work without this option. >> Is that the case here? If not, then please do not have this as the >> default. > Got it Greg. > > From my testing, the Raspberry Pi does seem to boot correctly without > this although some userspace libraries might not work. > > Since the machine itself works, I guess I'll drop this patch. The idea is good, but the patch needs improvement :) Acceptable solution might be that BCM2835_VCHIQ selects VCHIQ_CDEV. > > Thank you! > Ojaswin >> thanks, >> >> greg k-h > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jul 28, 2021 at 10:21:41PM +0200, Stefan Wahren wrote: > Am 28.07.21 um 22:06 schrieb Ojaswin Mujoo: > > On Wed, Jul 28, 2021 at 08:45:55PM +0200, Greg KH wrote: > >> On Thu, Jul 29, 2021 at 12:07:16AM +0530, Ojaswin Mujoo wrote: > >>> Before this config was defined, the cdev used to be created > >>> unconditionally. When an earlier commit introduced this config, the > >>> default behavior was set to disabled, which might surprise some > >>> unsuspecting users. Hence, make this config default to 'Y' to be more > >>> backward consistent. > >>> > >>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com> > >>> --- > >>> drivers/staging/vc04_services/Kconfig | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig > >>> index 63caa6818d37..2b70c37cdd09 100644 > >>> --- a/drivers/staging/vc04_services/Kconfig > >>> +++ b/drivers/staging/vc04_services/Kconfig > >>> @@ -23,6 +23,7 @@ if BCM2835_VCHIQ > >>> > >>> config VCHIQ_CDEV > >>> bool "VCHIQ Character Driver" > >>> + default y > >> > >> default y is only if the machine will not work without this option. > >> Is that the case here? If not, then please do not have this as the > >> default. > > Got it Greg. > > > > From my testing, the Raspberry Pi does seem to boot correctly without > > this although some userspace libraries might not work. > > > > Since the machine itself works, I guess I'll drop this patch. > > The idea is good, but the patch needs improvement :) > > Acceptable solution might be that BCM2835_VCHIQ selects VCHIQ_CDEV. Thanks for the suggestion, Stefan. So on testing with select, it seems like select defines a hard reverse dependency i.e if BCM2835_VCHIQ=y then VCHIQ_CDEV=y and it can't be set to n. Checking the Kconfig launguae doc, I found that we could use "imply" to define weak reverse dependencies, such that BCM2835_VCHIQ=y auto selects VCHIQ_CDEV=y but we can also change it back to n if required. I tested this out and it seems to work fine. I believe we can se imply here as it better suits the use case. Thank you, Ojaswin > > > > > Thank you! > > Ojaswin > >> thanks, > >> > >> greg k-h > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig index 63caa6818d37..2b70c37cdd09 100644 --- a/drivers/staging/vc04_services/Kconfig +++ b/drivers/staging/vc04_services/Kconfig @@ -23,6 +23,7 @@ if BCM2835_VCHIQ config VCHIQ_CDEV bool "VCHIQ Character Driver" + default y help Enable the creation of VCHIQ character driver to help communicate with the Videocore platform.
Before this config was defined, the cdev used to be created unconditionally. When an earlier commit introduced this config, the default behavior was set to disabled, which might surprise some unsuspecting users. Hence, make this config default to 'Y' to be more backward consistent. Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com> --- drivers/staging/vc04_services/Kconfig | 1 + 1 file changed, 1 insertion(+)