[PATCH 0/2] staging: vchiq: Minor fixups to CONFIG_VCHIQ_CDEV

[PATCH 0/2] staging: vchiq: Minor fixups to CONFIG_VCHIQ_CDEV

[Ojaswin Mujoo]

Hi everyone,

The patchset performs some minor fixups to the CONFIG_VCHIQ_CDEV config
option defined in /staging/vc04_services/Kconfig. (Introduced in [1])

Summary:

Patch 1: Enable the config by default to be more consistent with how
         cdev creation worked before this config was introduced
Patch 2: Add some more details to the config's help text

Please let me know if any changes are required.

Thank you!
Ojaswin

[1] https://lore.kernel.org/r/846c424dd4aae14d1cc28c8f30877a06e2b7dd10.1626882325.git.ojaswin98@gmail.com

Ojaswin Mujoo (2):
  staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default
  staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

 drivers/staging/vc04_services/Kconfig | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH 0/2] staging: vchiq: Minor fixups to CONFIG_VCHIQ_CDEV

[Ojaswin Mujoo]

Hi everyone,

The patchset performs some minor fixups to the CONFIG_VCHIQ_CDEV config
option defined in /staging/vc04_services/Kconfig. (Introduced in [1])

Summary:

Patch 1: Enable the config by default to be more consistent with how
         cdev creation worked before this config was introduced
Patch 2: Add some more details to the config's help text

Please let me know if any changes are required.

Thank you!
Ojaswin

[1] https://lore.kernel.org/r/846c424dd4aae14d1cc28c8f30877a06e2b7dd10.1626882325.git.ojaswin98@gmail.com

Ojaswin Mujoo (2):
  staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default
  staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

 drivers/staging/vc04_services/Kconfig | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Ojaswin Mujoo]

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
 	help
 		Enable the creation of VCHIQ character driver to help
 		communicate with the Videocore platform.
-- 
2.25.1

[PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Ojaswin Mujoo]

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
 	help
 		Enable the creation of VCHIQ character driver to help
 		communicate with the Videocore platform.
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
make the motive behind it a bit more clear.

Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
---
 drivers/staging/vc04_services/Kconfig | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
index 2b70c37cdd09..cb02d8a4cb74 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -25,8 +25,12 @@ config VCHIQ_CDEV
 	bool "VCHIQ Character Driver"
 	default y
 	help
-		Enable the creation of VCHIQ character driver to help
-		communicate with the Videocore platform.
+		Enable the creation of VCHIQ character driver to help communicate
+		with the VideoCore platform. The cdev exposes ioctls used by
+		userspace libraries and testing tools to interact with VideoCore.
+		This can be set to 'N' if the VideoCore communication is not needed
+		by userspace but only by other kernel modules (like bcm2835-audio).
+		If not sure, set this to 'Y'.
 
 endif
 
-- 
2.25.1

[PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
make the motive behind it a bit more clear.

Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
---
 drivers/staging/vc04_services/Kconfig | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
index 2b70c37cdd09..cb02d8a4cb74 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -25,8 +25,12 @@ config VCHIQ_CDEV
 	bool "VCHIQ Character Driver"
 	default y
 	help
-		Enable the creation of VCHIQ character driver to help
-		communicate with the Videocore platform.
+		Enable the creation of VCHIQ character driver to help communicate
+		with the VideoCore platform. The cdev exposes ioctls used by
+		userspace libraries and testing tools to interact with VideoCore.
+		This can be set to 'N' if the VideoCore communication is not needed
+		by userspace but only by other kernel modules (like bcm2835-audio).
+		If not sure, set this to 'Y'.
 
 endif
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Greg KH]

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

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Greg KH]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Greg KH]

On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> make the motive behind it a bit more clear.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> ---
>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> index 2b70c37cdd09..cb02d8a4cb74 100644
> --- a/drivers/staging/vc04_services/Kconfig
> +++ b/drivers/staging/vc04_services/Kconfig
> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
>  	bool "VCHIQ Character Driver"
>  	default y
>  	help
> -		Enable the creation of VCHIQ character driver to help
> -		communicate with the Videocore platform.
> +		Enable the creation of VCHIQ character driver to help communicate
> +		with the VideoCore platform. The cdev exposes ioctls used by
> +		userspace libraries and testing tools to interact with VideoCore.
> +		This can be set to 'N' if the VideoCore communication is not needed
> +		by userspace but only by other kernel modules (like bcm2835-audio).
> +		If not sure, set this to 'Y'.

I still do not understand if I need this driver or not, and I have this
hardware!  What functionality does this driver accomplish?  What is
VideoCore?

thanks,

greg k-h

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Greg KH]

On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> make the motive behind it a bit more clear.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> ---
>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> index 2b70c37cdd09..cb02d8a4cb74 100644
> --- a/drivers/staging/vc04_services/Kconfig
> +++ b/drivers/staging/vc04_services/Kconfig
> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
>  	bool "VCHIQ Character Driver"
>  	default y
>  	help
> -		Enable the creation of VCHIQ character driver to help
> -		communicate with the Videocore platform.
> +		Enable the creation of VCHIQ character driver to help communicate
> +		with the VideoCore platform. The cdev exposes ioctls used by
> +		userspace libraries and testing tools to interact with VideoCore.
> +		This can be set to 'N' if the VideoCore communication is not needed
> +		by userspace but only by other kernel modules (like bcm2835-audio).
> +		If not sure, set this to 'Y'.

I still do not understand if I need this driver or not, and I have this
hardware!  What functionality does this driver accomplish?  What is
VideoCore?

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> > Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> > make the motive behind it a bit more clear.
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> > ---
> >  drivers/staging/vc04_services/Kconfig | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> > index 2b70c37cdd09..cb02d8a4cb74 100644
> > --- a/drivers/staging/vc04_services/Kconfig
> > +++ b/drivers/staging/vc04_services/Kconfig
> > @@ -25,8 +25,12 @@ config VCHIQ_CDEV
> >  	bool "VCHIQ Character Driver"
> >  	default y
> >  	help
> > -		Enable the creation of VCHIQ character driver to help
> > -		communicate with the Videocore platform.
> > +		Enable the creation of VCHIQ character driver to help communicate
> > +		with the VideoCore platform. The cdev exposes ioctls used by
> > +		userspace libraries and testing tools to interact with VideoCore.
> > +		This can be set to 'N' if the VideoCore communication is not needed
> > +		by userspace but only by other kernel modules (like bcm2835-audio).
> > +		If not sure, set this to 'Y'.
> 
> I still do not understand if I need this driver or not, and I have this
> hardware!  What functionality does this driver accomplish?  What is
> VideoCore?
Hey Greg,

I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
as that enables the core driver that implements the functions to
communicate with VideoCore platform? 

This config option merely adds a cdev which exposes the the core
driver's functionality to userspace. 

Regards,
Ojaswin
> 
> thanks,
> 
> greg k-h

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> > Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> > make the motive behind it a bit more clear.
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> > ---
> >  drivers/staging/vc04_services/Kconfig | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> > index 2b70c37cdd09..cb02d8a4cb74 100644
> > --- a/drivers/staging/vc04_services/Kconfig
> > +++ b/drivers/staging/vc04_services/Kconfig
> > @@ -25,8 +25,12 @@ config VCHIQ_CDEV
> >  	bool "VCHIQ Character Driver"
> >  	default y
> >  	help
> > -		Enable the creation of VCHIQ character driver to help
> > -		communicate with the Videocore platform.
> > +		Enable the creation of VCHIQ character driver to help communicate
> > +		with the VideoCore platform. The cdev exposes ioctls used by
> > +		userspace libraries and testing tools to interact with VideoCore.
> > +		This can be set to 'N' if the VideoCore communication is not needed
> > +		by userspace but only by other kernel modules (like bcm2835-audio).
> > +		If not sure, set this to 'Y'.
> 
> I still do not understand if I need this driver or not, and I have this
> hardware!  What functionality does this driver accomplish?  What is
> VideoCore?
Hey Greg,

I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
as that enables the core driver that implements the functions to
communicate with VideoCore platform? 

This config option merely adds a cdev which exposes the the core
driver's functionality to userspace. 

Regards,
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

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[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.

Thank you!
Ojaswin
> 
> thanks,
> 
> greg k-h

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[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.

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

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Stefan Wahren]

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

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Stefan Wahren]

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Stefan Wahren]

Am 28.07.21 um 22:00 schrieb Ojaswin Mujoo:
> On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
>> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
>>> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
>>> make the motive behind it a bit more clear.
>>>
>>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
>>> ---
>>>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
>>> index 2b70c37cdd09..cb02d8a4cb74 100644
>>> --- a/drivers/staging/vc04_services/Kconfig
>>> +++ b/drivers/staging/vc04_services/Kconfig
>>> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
>>>  	bool "VCHIQ Character Driver"
>>>  	default y
>>>  	help
>>> -		Enable the creation of VCHIQ character driver to help
>>> -		communicate with the Videocore platform.
>>> +		Enable the creation of VCHIQ character driver to help communicate
>>> +		with the VideoCore platform. The cdev exposes ioctls used by
>>> +		userspace libraries and testing tools to interact with VideoCore.
>>> +		This can be set to 'N' if the VideoCore communication is not needed
>>> +		by userspace but only by other kernel modules (like bcm2835-audio).
>>> +		If not sure, set this to 'Y'.
>> I still do not understand if I need this driver or not, and I have this
>> hardware!  What functionality does this driver accomplish?  What is
>> VideoCore?
> Hey Greg,
>
> I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
> as that enables the core driver that implements the functions to
> communicate with VideoCore platform? 

Sorry, today i'm too tired to give a good explanation. VideoCore is the
VPU inside the BCM283x SoC. It runs a firmware and VCHIQ provides a way
to communicate with this firmware / VPU. The VCHIQ driver is required to
get access to the audio jack and camera interface (see depending drivers).

Unfortunately i don't have an application list by the hand which uses
the CDEV interface for VCHIQ, please use this link [1] as a starting point.

[1] - https://github.com/raspberrypi/userland

>
> This config option merely adds a cdev which exposes the the core
> driver's functionality to userspace. 
>
> Regards,
> 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

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Stefan Wahren]

Am 28.07.21 um 22:00 schrieb Ojaswin Mujoo:
> On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
>> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
>>> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
>>> make the motive behind it a bit more clear.
>>>
>>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
>>> ---
>>>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
>>> index 2b70c37cdd09..cb02d8a4cb74 100644
>>> --- a/drivers/staging/vc04_services/Kconfig
>>> +++ b/drivers/staging/vc04_services/Kconfig
>>> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
>>>  	bool "VCHIQ Character Driver"
>>>  	default y
>>>  	help
>>> -		Enable the creation of VCHIQ character driver to help
>>> -		communicate with the Videocore platform.
>>> +		Enable the creation of VCHIQ character driver to help communicate
>>> +		with the VideoCore platform. The cdev exposes ioctls used by
>>> +		userspace libraries and testing tools to interact with VideoCore.
>>> +		This can be set to 'N' if the VideoCore communication is not needed
>>> +		by userspace but only by other kernel modules (like bcm2835-audio).
>>> +		If not sure, set this to 'Y'.
>> I still do not understand if I need this driver or not, and I have this
>> hardware!  What functionality does this driver accomplish?  What is
>> VideoCore?
> Hey Greg,
>
> I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
> as that enables the core driver that implements the functions to
> communicate with VideoCore platform? 

Sorry, today i'm too tired to give a good explanation. VideoCore is the
VPU inside the BCM283x SoC. It runs a firmware and VCHIQ provides a way
to communicate with this firmware / VPU. The VCHIQ driver is required to
get access to the audio jack and camera interface (see depending drivers).

Unfortunately i don't have an application list by the hand which uses
the CDEV interface for VCHIQ, please use this link [1] as a starting point.

[1] - https://github.com/raspberrypi/userland

>
> This config option merely adds a cdev which exposes the the core
> driver's functionality to userspace. 
>
> Regards,
> 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

On Wed, Jul 28, 2021 at 10:39:38PM +0200, Stefan Wahren wrote:
> Am 28.07.21 um 22:00 schrieb Ojaswin Mujoo:
> > On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
> >> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> >>> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> >>> make the motive behind it a bit more clear.
> >>>
> >>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> >>> ---
> >>>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> >>> index 2b70c37cdd09..cb02d8a4cb74 100644
> >>> --- a/drivers/staging/vc04_services/Kconfig
> >>> +++ b/drivers/staging/vc04_services/Kconfig
> >>> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
> >>>  	bool "VCHIQ Character Driver"
> >>>  	default y
> >>>  	help
> >>> -		Enable the creation of VCHIQ character driver to help
> >>> -		communicate with the Videocore platform.
> >>> +		Enable the creation of VCHIQ character driver to help communicate
> >>> +		with the VideoCore platform. The cdev exposes ioctls used by
> >>> +		userspace libraries and testing tools to interact with VideoCore.
> >>> +		This can be set to 'N' if the VideoCore communication is not needed
> >>> +		by userspace but only by other kernel modules (like bcm2835-audio).
> >>> +		If not sure, set this to 'Y'.
> >> I still do not understand if I need this driver or not, and I have this
> >> hardware!  What functionality does this driver accomplish?  What is
> >> VideoCore?
> > Hey Greg,
> >
> > I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
> > as that enables the core driver that implements the functions to
> > communicate with VideoCore platform? 
> 
> Sorry, today i'm too tired to give a good explanation. VideoCore is the
> VPU inside the BCM283x SoC. It runs a firmware and VCHIQ provides a way
> to communicate with this firmware / VPU. The VCHIQ driver is required to
> get access to the audio jack and camera interface (see depending drivers).
> 
> Unfortunately i don't have an application list by the hand which uses
> the CDEV interface for VCHIQ, please use this link [1] as a starting point.
> 
> [1] - https://github.com/raspberrypi/userland
> 
Hi Stefan

Thanks for the brief, its good enough to help me get started. I've also
been looking more closely into this driver in my free time lately and I
can try to come up with a small summary to use as the help text. I'll
add it in v2 of this patch.

Regards,
Ojaswin
> >
> > This config option merely adds a cdev which exposes the the core
> > driver's functionality to userspace. 
> >
> > Regards,
> > 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
> 

Re: [PATCH 2/2] staging: vchiq: Add details to $CONFIG_VCHIQ_CDEV help text

[Ojaswin Mujoo]

On Wed, Jul 28, 2021 at 10:39:38PM +0200, Stefan Wahren wrote:
> Am 28.07.21 um 22:00 schrieb Ojaswin Mujoo:
> > On Wed, Jul 28, 2021 at 08:46:51PM +0200, Greg KH wrote:
> >> On Thu, Jul 29, 2021 at 12:07:17AM +0530, Ojaswin Mujoo wrote:
> >>> Add some details to the Kconfig definition of $CONFIG_VCHIQ_CDEV to help
> >>> make the motive behind it a bit more clear.
> >>>
> >>> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> >>> ---
> >>>  drivers/staging/vc04_services/Kconfig | 8 ++++++--
> >>>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/vc04_services/Kconfig b/drivers/staging/vc04_services/Kconfig
> >>> index 2b70c37cdd09..cb02d8a4cb74 100644
> >>> --- a/drivers/staging/vc04_services/Kconfig
> >>> +++ b/drivers/staging/vc04_services/Kconfig
> >>> @@ -25,8 +25,12 @@ config VCHIQ_CDEV
> >>>  	bool "VCHIQ Character Driver"
> >>>  	default y
> >>>  	help
> >>> -		Enable the creation of VCHIQ character driver to help
> >>> -		communicate with the Videocore platform.
> >>> +		Enable the creation of VCHIQ character driver to help communicate
> >>> +		with the VideoCore platform. The cdev exposes ioctls used by
> >>> +		userspace libraries and testing tools to interact with VideoCore.
> >>> +		This can be set to 'N' if the VideoCore communication is not needed
> >>> +		by userspace but only by other kernel modules (like bcm2835-audio).
> >>> +		If not sure, set this to 'Y'.
> >> I still do not understand if I need this driver or not, and I have this
> >> hardware!  What functionality does this driver accomplish?  What is
> >> VideoCore?
> > Hey Greg,
> >
> > I believe I can add this under the CONFIG_BCM2835_VCHIQ config option,
> > as that enables the core driver that implements the functions to
> > communicate with VideoCore platform? 
> 
> Sorry, today i'm too tired to give a good explanation. VideoCore is the
> VPU inside the BCM283x SoC. It runs a firmware and VCHIQ provides a way
> to communicate with this firmware / VPU. The VCHIQ driver is required to
> get access to the audio jack and camera interface (see depending drivers).
> 
> Unfortunately i don't have an application list by the hand which uses
> the CDEV interface for VCHIQ, please use this link [1] as a starting point.
> 
> [1] - https://github.com/raspberrypi/userland
> 
Hi Stefan

Thanks for the brief, its good enough to help me get started. I've also
been looking more closely into this driver in my free time lately and I
can try to come up with a small summary to use as the help text. I'll
add it in v2 of this patch.

Regards,
Ojaswin
> >
> > This config option merely adds a cdev which exposes the the core
> > driver's functionality to userspace. 
> >
> > Regards,
> > 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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Ojaswin Mujoo]

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
> 

Re: [PATCH 1/2] staging: vchiq: Set $CONFIG_VCHIQ_CDEV to be enabled by default

[Ojaswin Mujoo]

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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel