All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: vchiq: Update TODO file
@ 2021-09-27 18:13 ` Ojaswin Mujoo
  0 siblings, 0 replies; 6+ messages in thread
From: Ojaswin Mujoo @ 2021-09-27 18:13 UTC (permalink / raw)
  To: nsaenz, gregkh, stefan.wahren
  Cc: dan.carpenter, phil, linux-arm-kernel, linux-staging, linux-kernel

Remove completed tasks from TODO and add some relevant details to task
14.

Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
---
 drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 39810ce017cd..5a6b3fc2663a 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -16,23 +16,6 @@ some of the ones we want:
   to manage these buffers as dmabufs so that we can zero-copy import
   camera images into vc4 for rendering/display.
 
-2) Garbage-collect unused code
-
-One of the reasons this driver wasn't upstreamed previously was that
-there's a lot code that got built that's probably unnecessary these
-days.  Once we have the set of VCHI-using drivers we want in tree, we
-should be able to do a sweep of the code to see what's left that's
-unused.
-
-3) Make driver more portable
-
-Building this driver with arm/multi_v7_defconfig or arm64/defconfig
-leads to data corruption during the following command:
-
-  vchiq_test -f 1
-
-This should be fixed.
-
 4) Fix kernel module support
 
 Even the VPU firmware doesn't support a VCHI re-connect, the driver
@@ -68,22 +51,6 @@ The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
 indentation deep making it very unpleasant to read. This is specially relevant
 in the character driver ioctl code and in the core thread functions.
 
-10) Reorganize file structure: Move char driver to it's own file and join both
-platform files
-
-The cdev is defined alongside with the platform code in vchiq_arm.c. It would
-be nice to completely decouple it from the actual core code. For instance to be
-able to use bcm2835-audio without having /dev/vchiq created. One could argue
-it's better for security reasons or general cleanliness. It could even be
-interesting to create two different kernel modules, something the likes of
-vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
-
-The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
-
-12) Get rid of all the struct typedefs
-
-Most structs are typedefd, it's not encouraged in the kernel.
-
 13) Get rid of all non essential global structures and create a proper per
 device structure
 
@@ -92,6 +59,10 @@ for all the device specific data. This structure is then passed all over the
 driver. This is good practice since it makes the driver work regardless of the
 number of devices probed.
 
-14) Clean up Sparse warnings from __user annotations. See
-vchiq_irq_queue_bulk_tx_rx(). Ensure that the address of "&waiter->bulk_waiter"
-is never disclosed to userspace.
+14) Clean up Sparse warnings from __user annotations.
+
+This warning can be seen in vchiq_irq_queue_bulk_tx_rx(). Ensure that the
+address of "&waiter->bulk_waiter" is never disclosed to userspace. Refer the
+following link for a better insight into the issue:
+
+https://lore.kernel.org/lkml/YKxdCwsEb49Siwo6@zeniv-ca.linux.org.uk/T/#mc30d8c055c01952d2059873fedf6693948c0b1ca
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] staging: vchiq: Update TODO file
@ 2021-09-27 18:13 ` Ojaswin Mujoo
  0 siblings, 0 replies; 6+ messages in thread
From: Ojaswin Mujoo @ 2021-09-27 18:13 UTC (permalink / raw)
  To: nsaenz, gregkh, stefan.wahren
  Cc: dan.carpenter, phil, linux-arm-kernel, linux-staging, linux-kernel

Remove completed tasks from TODO and add some relevant details to task
14.

Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
---
 drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
index 39810ce017cd..5a6b3fc2663a 100644
--- a/drivers/staging/vc04_services/interface/TODO
+++ b/drivers/staging/vc04_services/interface/TODO
@@ -16,23 +16,6 @@ some of the ones we want:
   to manage these buffers as dmabufs so that we can zero-copy import
   camera images into vc4 for rendering/display.
 
-2) Garbage-collect unused code
-
-One of the reasons this driver wasn't upstreamed previously was that
-there's a lot code that got built that's probably unnecessary these
-days.  Once we have the set of VCHI-using drivers we want in tree, we
-should be able to do a sweep of the code to see what's left that's
-unused.
-
-3) Make driver more portable
-
-Building this driver with arm/multi_v7_defconfig or arm64/defconfig
-leads to data corruption during the following command:
-
-  vchiq_test -f 1
-
-This should be fixed.
-
 4) Fix kernel module support
 
 Even the VPU firmware doesn't support a VCHI re-connect, the driver
@@ -68,22 +51,6 @@ The code follows the 80 characters limitation yet tends to go 3 or 4 levels of
 indentation deep making it very unpleasant to read. This is specially relevant
 in the character driver ioctl code and in the core thread functions.
 
-10) Reorganize file structure: Move char driver to it's own file and join both
-platform files
-
-The cdev is defined alongside with the platform code in vchiq_arm.c. It would
-be nice to completely decouple it from the actual core code. For instance to be
-able to use bcm2835-audio without having /dev/vchiq created. One could argue
-it's better for security reasons or general cleanliness. It could even be
-interesting to create two different kernel modules, something the likes of
-vchiq-core.ko and vchiq-dev.ko. This would also ease the upstreaming process.
-
-The code in vchiq_bcm2835_arm.c should fit in the generic platform file.
-
-12) Get rid of all the struct typedefs
-
-Most structs are typedefd, it's not encouraged in the kernel.
-
 13) Get rid of all non essential global structures and create a proper per
 device structure
 
@@ -92,6 +59,10 @@ for all the device specific data. This structure is then passed all over the
 driver. This is good practice since it makes the driver work regardless of the
 number of devices probed.
 
-14) Clean up Sparse warnings from __user annotations. See
-vchiq_irq_queue_bulk_tx_rx(). Ensure that the address of "&waiter->bulk_waiter"
-is never disclosed to userspace.
+14) Clean up Sparse warnings from __user annotations.
+
+This warning can be seen in vchiq_irq_queue_bulk_tx_rx(). Ensure that the
+address of "&waiter->bulk_waiter" is never disclosed to userspace. Refer the
+following link for a better insight into the issue:
+
+https://lore.kernel.org/lkml/YKxdCwsEb49Siwo6@zeniv-ca.linux.org.uk/T/#mc30d8c055c01952d2059873fedf6693948c0b1ca
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: vchiq: Update TODO file
  2021-09-27 18:13 ` Ojaswin Mujoo
@ 2021-09-28  7:10   ` Greg KH
  -1 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-09-28  7:10 UTC (permalink / raw)
  To: Ojaswin Mujoo
  Cc: nsaenz, stefan.wahren, dan.carpenter, phil, linux-arm-kernel,
	linux-staging, linux-kernel

On Mon, Sep 27, 2021 at 11:43:23PM +0530, Ojaswin Mujoo wrote:
> Remove completed tasks from TODO and add some relevant details to task
> 14.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> ---
>  drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
>  1 file changed, 7 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> index 39810ce017cd..5a6b3fc2663a 100644
> --- a/drivers/staging/vc04_services/interface/TODO
> +++ b/drivers/staging/vc04_services/interface/TODO
> @@ -16,23 +16,6 @@ some of the ones we want:
>    to manage these buffers as dmabufs so that we can zero-copy import
>    camera images into vc4 for rendering/display.
>  
> -2) Garbage-collect unused code
> -
> -One of the reasons this driver wasn't upstreamed previously was that
> -there's a lot code that got built that's probably unnecessary these
> -days.  Once we have the set of VCHI-using drivers we want in tree, we
> -should be able to do a sweep of the code to see what's left that's
> -unused.
> -
> -3) Make driver more portable
> -
> -Building this driver with arm/multi_v7_defconfig or arm64/defconfig
> -leads to data corruption during the following command:
> -
> -  vchiq_test -f 1
> -
> -This should be fixed.
> -
>  4) Fix kernel module support

Now the numbering is out of order?

How about a first patch to remove the numbering and just replace it with
a list of items that are not numbered, and then remove the tasks that
are already done?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: vchiq: Update TODO file
@ 2021-09-28  7:10   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2021-09-28  7:10 UTC (permalink / raw)
  To: Ojaswin Mujoo
  Cc: nsaenz, stefan.wahren, dan.carpenter, phil, linux-arm-kernel,
	linux-staging, linux-kernel

On Mon, Sep 27, 2021 at 11:43:23PM +0530, Ojaswin Mujoo wrote:
> Remove completed tasks from TODO and add some relevant details to task
> 14.
> 
> Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> ---
>  drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
>  1 file changed, 7 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> index 39810ce017cd..5a6b3fc2663a 100644
> --- a/drivers/staging/vc04_services/interface/TODO
> +++ b/drivers/staging/vc04_services/interface/TODO
> @@ -16,23 +16,6 @@ some of the ones we want:
>    to manage these buffers as dmabufs so that we can zero-copy import
>    camera images into vc4 for rendering/display.
>  
> -2) Garbage-collect unused code
> -
> -One of the reasons this driver wasn't upstreamed previously was that
> -there's a lot code that got built that's probably unnecessary these
> -days.  Once we have the set of VCHI-using drivers we want in tree, we
> -should be able to do a sweep of the code to see what's left that's
> -unused.
> -
> -3) Make driver more portable
> -
> -Building this driver with arm/multi_v7_defconfig or arm64/defconfig
> -leads to data corruption during the following command:
> -
> -  vchiq_test -f 1
> -
> -This should be fixed.
> -
>  4) Fix kernel module support

Now the numbering is out of order?

How about a first patch to remove the numbering and just replace it with
a list of items that are not numbered, and then remove the tasks that
are already done?

thanks,

greg k-h

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: vchiq: Update TODO file
  2021-09-28  7:10   ` Greg KH
@ 2021-09-30 16:49     ` Ojaswin Mujoo
  -1 siblings, 0 replies; 6+ messages in thread
From: Ojaswin Mujoo @ 2021-09-30 16:49 UTC (permalink / raw)
  To: Greg KH
  Cc: nsaenz, stefan.wahren, dan.carpenter, phil, linux-arm-kernel,
	linux-staging, linux-kernel

On Tue, Sep 28, 2021 at 09:10:44AM +0200, Greg KH wrote:
> On Mon, Sep 27, 2021 at 11:43:23PM +0530, Ojaswin Mujoo wrote:
> > Remove completed tasks from TODO and add some relevant details to task
> > 14.
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> > ---
> >  drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
> >  1 file changed, 7 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> > index 39810ce017cd..5a6b3fc2663a 100644
> > --- a/drivers/staging/vc04_services/interface/TODO
> > +++ b/drivers/staging/vc04_services/interface/TODO
> > @@ -16,23 +16,6 @@ some of the ones we want:
> >    to manage these buffers as dmabufs so that we can zero-copy import
> >    camera images into vc4 for rendering/display.
> >  
> > -2) Garbage-collect unused code
> > -
> > -One of the reasons this driver wasn't upstreamed previously was that
> > -there's a lot code that got built that's probably unnecessary these
> > -days.  Once we have the set of VCHI-using drivers we want in tree, we
> > -should be able to do a sweep of the code to see what's left that's
> > -unused.
> > -
> > -3) Make driver more portable
> > -
> > -Building this driver with arm/multi_v7_defconfig or arm64/defconfig
> > -leads to data corruption during the following command:
> > -
> > -  vchiq_test -f 1
> > -
> > -This should be fixed.
> > -
> >  4) Fix kernel module support
Hi Greg,
> 
> Now the numbering is out of order?
It was already out of order (No point 11). I assumed it was so that references to the
tasks nos in discussion that took place before this patch will make sense.
> 
> How about a first patch to remove the numbering and just replace it with
> a list of items that are not numbered, and then remove the tasks that
> are already done?
Sure I can do that.

Thanks for the review.
ojaswin
> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: vchiq: Update TODO file
@ 2021-09-30 16:49     ` Ojaswin Mujoo
  0 siblings, 0 replies; 6+ messages in thread
From: Ojaswin Mujoo @ 2021-09-30 16:49 UTC (permalink / raw)
  To: Greg KH
  Cc: nsaenz, stefan.wahren, dan.carpenter, phil, linux-arm-kernel,
	linux-staging, linux-kernel

On Tue, Sep 28, 2021 at 09:10:44AM +0200, Greg KH wrote:
> On Mon, Sep 27, 2021 at 11:43:23PM +0530, Ojaswin Mujoo wrote:
> > Remove completed tasks from TODO and add some relevant details to task
> > 14.
> > 
> > Signed-off-by: Ojaswin Mujoo <ojaswin98@gmail.com>
> > ---
> >  drivers/staging/vc04_services/interface/TODO | 43 ++++----------------
> >  1 file changed, 7 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/staging/vc04_services/interface/TODO b/drivers/staging/vc04_services/interface/TODO
> > index 39810ce017cd..5a6b3fc2663a 100644
> > --- a/drivers/staging/vc04_services/interface/TODO
> > +++ b/drivers/staging/vc04_services/interface/TODO
> > @@ -16,23 +16,6 @@ some of the ones we want:
> >    to manage these buffers as dmabufs so that we can zero-copy import
> >    camera images into vc4 for rendering/display.
> >  
> > -2) Garbage-collect unused code
> > -
> > -One of the reasons this driver wasn't upstreamed previously was that
> > -there's a lot code that got built that's probably unnecessary these
> > -days.  Once we have the set of VCHI-using drivers we want in tree, we
> > -should be able to do a sweep of the code to see what's left that's
> > -unused.
> > -
> > -3) Make driver more portable
> > -
> > -Building this driver with arm/multi_v7_defconfig or arm64/defconfig
> > -leads to data corruption during the following command:
> > -
> > -  vchiq_test -f 1
> > -
> > -This should be fixed.
> > -
> >  4) Fix kernel module support
Hi Greg,
> 
> Now the numbering is out of order?
It was already out of order (No point 11). I assumed it was so that references to the
tasks nos in discussion that took place before this patch will make sense.
> 
> How about a first patch to remove the numbering and just replace it with
> a list of items that are not numbered, and then remove the tasks that
> are already done?
Sure I can do that.

Thanks for the review.
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-30 16:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 18:13 [PATCH] staging: vchiq: Update TODO file Ojaswin Mujoo
2021-09-27 18:13 ` Ojaswin Mujoo
2021-09-28  7:10 ` Greg KH
2021-09-28  7:10   ` Greg KH
2021-09-30 16:49   ` Ojaswin Mujoo
2021-09-30 16:49     ` Ojaswin Mujoo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.