Re: [PATCH] Staging: most: Remove volatile usage
> Since it is _not_ assured that the code will run under all circumstances, > I don't want this patch to be applied at this point. > > To be able to decide on the removal of the volatile keyword, we need to run > some tests on real hardware. This ain't something we can tell by just > looking at the code. > > We'll add this to our todo list. Thanks Christian, it will be really helpful. Thanks, PrasannaKumar ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: most: Remove volatile usage
On Mon, 21 Mar 2016 15:36:54 -0400 Greg KH wrote: > On Mon, Mar 21, 2016 at 03:08:41PM +0530, PrasannaKumar Muralidharan wrote: > > > Are you sure you can just remove these markings? Does the code work the > > > same? What is properly locking these values? Why were they marked this > > > way in the first place? > > > > I could not test the change due to lack of hardware. > > > > Given that the code works without lock and as volatile does not > > guarantee any sort of locking I understand that lock is not necessary. > > Please correct me if I am wrong. > > > > Could not find the purpose of 'volatile' in the code, > > 'request_counter' and 'service_counter' are only accessed by the CPU. > > So I think it can be removed. > > I will need the maintainers of the code to ack this before I can accept > it... > Since it is _not_ assured that the code will run under all circumstances, I don't want this patch to be applied at this point. To be able to decide on the removal of the volatile keyword, we need to run some tests on real hardware. This ain't something we can tell by just looking at the code. We'll add this to our todo list. Regards, Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: most: Remove volatile usage
On Mon, Mar 21, 2016 at 03:08:41PM +0530, PrasannaKumar Muralidharan wrote: > > Are you sure you can just remove these markings? Does the code work the > > same? What is properly locking these values? Why were they marked this > > way in the first place? > > I could not test the change due to lack of hardware. > > Given that the code works without lock and as volatile does not > guarantee any sort of locking I understand that lock is not necessary. > Please correct me if I am wrong. > > Could not find the purpose of 'volatile' in the code, > 'request_counter' and 'service_counter' are only accessed by the CPU. > So I think it can be removed. I will need the maintainers of the code to ack this before I can accept it... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: most: Remove volatile usage
> Are you sure you can just remove these markings? Does the code work the > same? What is properly locking these values? Why were they marked this > way in the first place? I could not test the change due to lack of hardware. Given that the code works without lock and as volatile does not guarantee any sort of locking I understand that lock is not necessary. Please correct me if I am wrong. Could not find the purpose of 'volatile' in the code, 'request_counter' and 'service_counter' are only accessed by the CPU. So I think it can be removed. Thanks, PrasannaKumar ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: most: Remove volatile usage
On Sat, Mar 12, 2016 at 02:11:10PM +0530, PrasannaKumar Muralidharan wrote:
> From: PrasannaKumar Muralidharan
>
> Remove unnecessary use of volatile for 'request_counter' and
> 'service_counter' members.
>
> Signed-off-by: PrasannaKumar Muralidharan
> ---
> drivers/staging/most/hdm-dim2/dim2_hal.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/most/hdm-dim2/dim2_hal.h
> b/drivers/staging/most/hdm-dim2/dim2_hal.h
> index 1c924e8..ee10ed9 100644
> --- a/drivers/staging/most/hdm-dim2/dim2_hal.h
> +++ b/drivers/staging/most/hdm-dim2/dim2_hal.h
> @@ -40,10 +40,10 @@ struct dim_ch_state_t {
>
> struct int_ch_state {
> /* changed only in interrupt context */
> - volatile int request_counter;
> + int request_counter;
Are you sure you can just remove these markings? Does the code work the
same? What is properly locking these values? Why were they marked this
way in the first place?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
