* [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c @ 2026-03-27 3:11 Anushka Badhe 2026-03-27 9:43 ` Andy Shevchenko 2026-03-27 11:24 ` Sakari Ailus 0 siblings, 2 replies; 9+ messages in thread From: Anushka Badhe @ 2026-03-27 3:11 UTC (permalink / raw) To: andriy.shevchenko Cc: andy, anushkabadhe, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus GP_TIMER_BASE is only used in gp_timer.c and it does not need to be globally visible. Move its declaration from system_local.c to gp_timer.c and make it file local by marking it static. Remove external declaration from system_local.h and its usage in gp_timer.h This fixes a sparse warning about global visibility and cleans up unnecessary global exposure. Signed-off-by: Anushka Badhe <anushkabadhe@gmail.com> --- Changes in v6: - Mark scope of GP_TIMER_BASE static Changes in v5: - Move GP_TIMER_BASE definition to gp_timer.c - Remove extern from system_local.h - Remove include of system_local.h from gp_timer.h Changes in v4: - Remove unrelated block comment style fixes Changes in v3: - Add commit description - Fix subject prefix to staging: media: atomisp: Changes in v2: - Fix block comment style (move closing */ to its own line) - Merge split GP_TIMER_BASE declaration onto a single line Note: * This patch is part of the GSoC2026 application process for device tree binding s conversions * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings .../media/atomisp/pci/hive_isp_css_common/host/gp_timer.c | 7 ++++++- .../media/atomisp/pci/hive_isp_css_include/gp_timer.h | 1 - drivers/staging/media/atomisp/pci/system_local.c | 6 ------ drivers/staging/media/atomisp/pci/system_local.h | 5 ----- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c index d04c179a5ecd..0c1b67988dd9 100644 --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c @@ -11,7 +11,12 @@ #ifndef __INLINE_GP_TIMER__ #include "gp_timer_private.h" /*device_access.h*/ #endif /* __INLINE_GP_TIMER__ */ -#include "system_local.h" + +/*GP TIMER , all timer registers are inter-twined, + * so, having multiple base addresses for + * different timers does not help + */ +static const hrt_address GP_TIMER_BASE = (hrt_address)0x0000000000000600ULL; /* FIXME: not sure if reg_load(), reg_store() should be API. */ diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h index 94f81af70007..e651d9ef1114 100644 --- a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h @@ -21,7 +21,6 @@ * - local: system and cell specific constants and identifiers */ -#include "system_local.h" /*GP_TIMER_BASE address */ #include "gp_timer_local.h" /*GP_TIMER register offsets */ #ifndef __INLINE_GP_TIMER__ diff --git a/drivers/staging/media/atomisp/pci/system_local.c b/drivers/staging/media/atomisp/pci/system_local.c index a8a93760d5b1..8d4fd80f8984 100644 --- a/drivers/staging/media/atomisp/pci/system_local.c +++ b/drivers/staging/media/atomisp/pci/system_local.c @@ -83,12 +83,6 @@ const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID] = { 0x0000000000000000ULL }; -/*GP TIMER , all timer registers are inter-twined, - * so, having multiple base addresses for - * different timers does not help*/ -const hrt_address GP_TIMER_BASE = - (hrt_address)0x0000000000000600ULL; - /* GPIO */ const hrt_address GPIO_BASE[N_GPIO_ID] = { 0x0000000000000400ULL diff --git a/drivers/staging/media/atomisp/pci/system_local.h b/drivers/staging/media/atomisp/pci/system_local.h index 970f4ef990ec..2bd46f5123fb 100644 --- a/drivers/staging/media/atomisp/pci/system_local.h +++ b/drivers/staging/media/atomisp/pci/system_local.h @@ -53,11 +53,6 @@ extern const hrt_address FIFO_MONITOR_BASE[N_FIFO_MONITOR_ID]; /* GP_DEVICE (single base for all separate GP_REG instances) */ extern const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID]; -/*GP TIMER , all timer registers are inter-twined, - * so, having multiple base addresses for - * different timers does not help*/ -extern const hrt_address GP_TIMER_BASE; - /* GPIO */ extern const hrt_address GPIO_BASE[N_GPIO_ID]; -- 2.43.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-27 3:11 [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c Anushka Badhe @ 2026-03-27 9:43 ` Andy Shevchenko 2026-03-30 1:19 ` Anushka B 2026-03-27 11:24 ` Sakari Ailus 1 sibling, 1 reply; 9+ messages in thread From: Andy Shevchenko @ 2026-03-27 9:43 UTC (permalink / raw) To: Anushka Badhe Cc: andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus On Fri, Mar 27, 2026 at 08:41:06AM +0530, Anushka Badhe wrote: > GP_TIMER_BASE is only used in gp_timer.c and it does not need to be > globally visible. > > Move its declaration from system_local.c to gp_timer.c and make it file > local by marking it static. Remove external declaration from system_local.h > and its usage in gp_timer.h > > This fixes a sparse warning about global visibility and cleans up > unnecessary global exposure. ... > --- > Changes in v6: > - Mark scope of GP_TIMER_BASE static Good catch, but read my comment against v5. And slow down with new versions, no new version within 24h, please! -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-27 9:43 ` Andy Shevchenko @ 2026-03-30 1:19 ` Anushka B 2026-03-30 8:47 ` Andy Shevchenko 0 siblings, 1 reply; 9+ messages in thread From: Anushka B @ 2026-03-30 1:19 UTC (permalink / raw) To: Andy Shevchenko Cc: andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus, Daniel Baluta, Dhruva gole, Simona Toaca, m-chawdhry On Fri, Mar 27, 2026 at 3:13 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Fri, Mar 27, 2026 at 08:41:06AM +0530, Anushka Badhe wrote: > > GP_TIMER_BASE is only used in gp_timer.c and it does not need to be > > globally visible. > > > > Move its declaration from system_local.c to gp_timer.c and make it file > > local by marking it static. Remove external declaration from system_local.h > > and its usage in gp_timer.h > > > > This fixes a sparse warning about global visibility and cleans up > > unnecessary global exposure. > > ... > > > --- > > Changes in v6: > > - Mark scope of GP_TIMER_BASE static > > Good catch, but read my comment against v5. And slow down with new versions, > no new version within 24h, please! > > -- > With Best Regards, > Andy Shevchenko > Hi Andy, I saw your comment on v5 regarding the GP TIMER comment style, thank you for the example. I also received feedback from Sakari on this file: > Please don't move the defition here. There's a reason for keeping it in the > same location with the rest of the offsets. Would it be acceptable to go back to the simpler in-place merge in system_local.c, joining the split declaration of GP_TIMER_BASE? I can follow up with the comment formatting as a separate patch. I'll make sure to discuss changes before sending new revisions going forward. Thank you, Anushka ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-30 1:19 ` Anushka B @ 2026-03-30 8:47 ` Andy Shevchenko 0 siblings, 0 replies; 9+ messages in thread From: Andy Shevchenko @ 2026-03-30 8:47 UTC (permalink / raw) To: Anushka B Cc: Andy Shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus, Daniel Baluta, Dhruva gole, Simona Toaca, m-chawdhry On Mon, Mar 30, 2026 at 4:19 AM Anushka B <anushkabadhe@gmail.com> wrote: > On Fri, Mar 27, 2026 at 3:13 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Fri, Mar 27, 2026 at 08:41:06AM +0530, Anushka Badhe wrote: ... > > > --- > > > Changes in v6: > > > - Mark scope of GP_TIMER_BASE static > > > > Good catch, but read my comment against v5. And slow down with new versions, > > no new version within 24h, please! > I saw your comment on v5 regarding the GP TIMER comment style, thank > you for the example. > I also received feedback from Sakari on this file: > > Please don't move the defition here. There's a reason for keeping it in the > > same location with the rest of the offsets. > Would it be acceptable to go back to the simpler in-place merge in > system_local.c, joining the split > declaration of GP_TIMER_BASE? I can follow up with the comment > formatting as a separate patch. Sakari is the maintainer, follow what he says. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-27 3:11 [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c Anushka Badhe 2026-03-27 9:43 ` Andy Shevchenko @ 2026-03-27 11:24 ` Sakari Ailus 2026-03-30 1:32 ` Anushka B 1 sibling, 1 reply; 9+ messages in thread From: Sakari Ailus @ 2026-03-27 11:24 UTC (permalink / raw) To: Anushka Badhe Cc: andriy.shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab Hi Anushka, Thanks for the update. On Fri, Mar 27, 2026 at 08:41:06AM +0530, Anushka Badhe wrote: > GP_TIMER_BASE is only used in gp_timer.c and it does not need to be > globally visible. > > Move its declaration from system_local.c to gp_timer.c and make it file > local by marking it static. Remove external declaration from system_local.h > and its usage in gp_timer.h > > This fixes a sparse warning about global visibility and cleans up > unnecessary global exposure. > > Signed-off-by: Anushka Badhe <anushkabadhe@gmail.com> > --- > Changes in v6: > - Mark scope of GP_TIMER_BASE static > > Changes in v5: > - Move GP_TIMER_BASE definition to gp_timer.c > - Remove extern from system_local.h > - Remove include of system_local.h from gp_timer.h > > Changes in v4: > - Remove unrelated block comment style fixes > > Changes in v3: > - Add commit description > - Fix subject prefix to staging: media: atomisp: > > Changes in v2: > - Fix block comment style (move closing */ to its own line) > - Merge split GP_TIMER_BASE declaration onto a single line > > Note: > * This patch is part of the GSoC2026 application process for device tree > binding > s conversions > * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings > > .../media/atomisp/pci/hive_isp_css_common/host/gp_timer.c | 7 ++++++- > .../media/atomisp/pci/hive_isp_css_include/gp_timer.h | 1 - > drivers/staging/media/atomisp/pci/system_local.c | 6 ------ > drivers/staging/media/atomisp/pci/system_local.h | 5 ----- > 4 files changed, 6 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > index d04c179a5ecd..0c1b67988dd9 100644 > --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > @@ -11,7 +11,12 @@ > #ifndef __INLINE_GP_TIMER__ > #include "gp_timer_private.h" /*device_access.h*/ > #endif /* __INLINE_GP_TIMER__ */ > -#include "system_local.h" > + > +/*GP TIMER , all timer registers are inter-twined, > + * so, having multiple base addresses for > + * different timers does not help > + */ > +static const hrt_address GP_TIMER_BASE = (hrt_address)0x0000000000000600ULL; Please don't move the defition here. There's a reason for keeping it in the same location with the rest of the offsets. There's a lot to cleanup here but what should be done is roughly: - Make these constants macros (with IPU2_ or ATOMISP2_ prefix?) and move them into a separate header (perhaps with register definitions?). - Remove my_env and make struct device (or maybe struct atomisp_device?) as a parameter for register access functions. This may get a bit complicated due to the amount of cleanup needed so having the hardware for testing would be rather essential. > > /* FIXME: not sure if reg_load(), reg_store() should be API. > */ > diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > index 94f81af70007..e651d9ef1114 100644 > --- a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > @@ -21,7 +21,6 @@ > * - local: system and cell specific constants and identifiers > */ > > -#include "system_local.h" /*GP_TIMER_BASE address */ > #include "gp_timer_local.h" /*GP_TIMER register offsets */ > > #ifndef __INLINE_GP_TIMER__ > diff --git a/drivers/staging/media/atomisp/pci/system_local.c b/drivers/staging/media/atomisp/pci/system_local.c > index a8a93760d5b1..8d4fd80f8984 100644 > --- a/drivers/staging/media/atomisp/pci/system_local.c > +++ b/drivers/staging/media/atomisp/pci/system_local.c > @@ -83,12 +83,6 @@ const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID] = { > 0x0000000000000000ULL > }; > > -/*GP TIMER , all timer registers are inter-twined, > - * so, having multiple base addresses for > - * different timers does not help*/ This comment could benefit from fixing, regarding both formatting and language. > -const hrt_address GP_TIMER_BASE = > - (hrt_address)0x0000000000000600ULL; > - > /* GPIO */ > const hrt_address GPIO_BASE[N_GPIO_ID] = { > 0x0000000000000400ULL > diff --git a/drivers/staging/media/atomisp/pci/system_local.h b/drivers/staging/media/atomisp/pci/system_local.h > index 970f4ef990ec..2bd46f5123fb 100644 > --- a/drivers/staging/media/atomisp/pci/system_local.h > +++ b/drivers/staging/media/atomisp/pci/system_local.h > @@ -53,11 +53,6 @@ extern const hrt_address FIFO_MONITOR_BASE[N_FIFO_MONITOR_ID]; > /* GP_DEVICE (single base for all separate GP_REG instances) */ > extern const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID]; > > -/*GP TIMER , all timer registers are inter-twined, > - * so, having multiple base addresses for > - * different timers does not help*/ > -extern const hrt_address GP_TIMER_BASE; > - > /* GPIO */ > extern const hrt_address GPIO_BASE[N_GPIO_ID]; > -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-27 11:24 ` Sakari Ailus @ 2026-03-30 1:32 ` Anushka B 2026-04-01 6:55 ` Sakari Ailus 0 siblings, 1 reply; 9+ messages in thread From: Anushka B @ 2026-03-30 1:32 UTC (permalink / raw) To: Sakari Ailus Cc: andriy.shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, Daniel Baluta, Dhruva gole, m-chawdhry, Simona Toaca On Fri, Mar 27, 2026 at 4:54 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Anushka, > > Thanks for the update. > > On Fri, Mar 27, 2026 at 08:41:06AM +0530, Anushka Badhe wrote: > > GP_TIMER_BASE is only used in gp_timer.c and it does not need to be > > globally visible. > > > > Move its declaration from system_local.c to gp_timer.c and make it file > > local by marking it static. Remove external declaration from system_local.h > > and its usage in gp_timer.h > > > > This fixes a sparse warning about global visibility and cleans up > > unnecessary global exposure. > > > > Signed-off-by: Anushka Badhe <anushkabadhe@gmail.com> > > --- > > Changes in v6: > > - Mark scope of GP_TIMER_BASE static > > > > Changes in v5: > > - Move GP_TIMER_BASE definition to gp_timer.c > > - Remove extern from system_local.h > > - Remove include of system_local.h from gp_timer.h > > > > Changes in v4: > > - Remove unrelated block comment style fixes > > > > Changes in v3: > > - Add commit description > > - Fix subject prefix to staging: media: atomisp: > > > > Changes in v2: > > - Fix block comment style (move closing */ to its own line) > > - Merge split GP_TIMER_BASE declaration onto a single line > > > > Note: > > * This patch is part of the GSoC2026 application process for device tree > > binding > > s conversions > > * https://github.com/LinuxFoundationGSoC/ProjectIdeas/wiki/GSoC-2026-Device-Tree-Bindings > > > > .../media/atomisp/pci/hive_isp_css_common/host/gp_timer.c | 7 ++++++- > > .../media/atomisp/pci/hive_isp_css_include/gp_timer.h | 1 - > > drivers/staging/media/atomisp/pci/system_local.c | 6 ------ > > drivers/staging/media/atomisp/pci/system_local.h | 5 ----- > > 4 files changed, 6 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > > index d04c179a5ecd..0c1b67988dd9 100644 > > --- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > > +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/gp_timer.c > > @@ -11,7 +11,12 @@ > > #ifndef __INLINE_GP_TIMER__ > > #include "gp_timer_private.h" /*device_access.h*/ > > #endif /* __INLINE_GP_TIMER__ */ > > -#include "system_local.h" > > + > > +/*GP TIMER , all timer registers are inter-twined, > > + * so, having multiple base addresses for > > + * different timers does not help > > + */ > > +static const hrt_address GP_TIMER_BASE = (hrt_address)0x0000000000000600ULL; > > Please don't move the defition here. There's a reason for keeping it in the > same location with the rest of the offsets. There's a lot to cleanup here > but what should be done is roughly: > > - Make these constants macros (with IPU2_ or ATOMISP2_ prefix?) and move > them into a separate header (perhaps with register definitions?). > > - Remove my_env and make struct device (or maybe struct atomisp_device?) as > a parameter for register access functions. > > This may get a bit complicated due to the amount of cleanup needed so > having the hardware for testing would be rather essential. > > > > > /* FIXME: not sure if reg_load(), reg_store() should be API. > > */ > > diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > > index 94f81af70007..e651d9ef1114 100644 > > --- a/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > > +++ b/drivers/staging/media/atomisp/pci/hive_isp_css_include/gp_timer.h > > @@ -21,7 +21,6 @@ > > * - local: system and cell specific constants and identifiers > > */ > > > > -#include "system_local.h" /*GP_TIMER_BASE address */ > > #include "gp_timer_local.h" /*GP_TIMER register offsets */ > > > > #ifndef __INLINE_GP_TIMER__ > > diff --git a/drivers/staging/media/atomisp/pci/system_local.c b/drivers/staging/media/atomisp/pci/system_local.c > > index a8a93760d5b1..8d4fd80f8984 100644 > > --- a/drivers/staging/media/atomisp/pci/system_local.c > > +++ b/drivers/staging/media/atomisp/pci/system_local.c > > @@ -83,12 +83,6 @@ const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID] = { > > 0x0000000000000000ULL > > }; > > > > -/*GP TIMER , all timer registers are inter-twined, > > - * so, having multiple base addresses for > > - * different timers does not help*/ > > This comment could benefit from fixing, regarding both formatting and > language. > > > -const hrt_address GP_TIMER_BASE = > > - (hrt_address)0x0000000000000600ULL; > > - > > /* GPIO */ > > const hrt_address GPIO_BASE[N_GPIO_ID] = { > > 0x0000000000000400ULL > > diff --git a/drivers/staging/media/atomisp/pci/system_local.h b/drivers/staging/media/atomisp/pci/system_local.h > > index 970f4ef990ec..2bd46f5123fb 100644 > > --- a/drivers/staging/media/atomisp/pci/system_local.h > > +++ b/drivers/staging/media/atomisp/pci/system_local.h > > @@ -53,11 +53,6 @@ extern const hrt_address FIFO_MONITOR_BASE[N_FIFO_MONITOR_ID]; > > /* GP_DEVICE (single base for all separate GP_REG instances) */ > > extern const hrt_address GP_DEVICE_BASE[N_GP_DEVICE_ID]; > > > > -/*GP TIMER , all timer registers are inter-twined, > > - * so, having multiple base addresses for > > - * different timers does not help*/ > > -extern const hrt_address GP_TIMER_BASE; > > - > > /* GPIO */ > > extern const hrt_address GPIO_BASE[N_GPIO_ID]; > > > > -- > Kind regards, > > Sakari Ailus Hi Sakari, Thank you for your reply. > There's a lot to cleanup here, but what should be done is roughly: > > - Make these constants macros (with IPU2_ or ATOMISP2_ prefix?) and move > them into a separate header (perhaps with register definitions?). > > - Remove my_env and make struct device (or maybe struct atomisp_device?) as > a parameter for register access functions. > > This may get a bit complicated due to the amount of cleanup needed so > having the hardware for testing would be rather essential. As a student, I don't currently have access to the hardware needed to do the bigger refactor safely. However I'm happy to fix the comment formatting and language. In earlier versions, I had merged the split declaration of GP_TIMER_BASE in place in system_local.c before later moving it to gp_timer.c. Link: https://lore.kernel.org/linux-media/20260325132434.55775-1-anushkabadhe@gmail.com/ Would it be okay to proceed with the simpler in-place fix for GP_TIMER_BASE declaration instead? Thank you, Anushka ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-03-30 1:32 ` Anushka B @ 2026-04-01 6:55 ` Sakari Ailus 2026-04-01 16:02 ` Anushka B 0 siblings, 1 reply; 9+ messages in thread From: Sakari Ailus @ 2026-04-01 6:55 UTC (permalink / raw) To: Anushka B Cc: andriy.shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, Daniel Baluta, Dhruva gole, m-chawdhry, Simona Toaca Hi Anushka, On Mon, Mar 30, 2026 at 07:02:21AM +0530, Anushka B wrote: > > There's a lot to cleanup here, but what should be done is roughly: > > > > - Make these constants macros (with IPU2_ or ATOMISP2_ prefix?) and move > > them into a separate header (perhaps with register definitions?). > > > > - Remove my_env and make struct device (or maybe struct atomisp_device?) as > > a parameter for register access functions. > > > > This may get a bit complicated due to the amount of cleanup needed so > > having the hardware for testing would be rather essential. > As a student, I don't currently have access to the hardware needed to > do the bigger refactor safely. However I'm happy to fix the comment > formatting and language. > In earlier versions, I had merged the split declaration of > GP_TIMER_BASE in place in system_local.c before later moving it to > gp_timer.c. > Link: https://lore.kernel.org/linux-media/20260325132434.55775-1-anushkabadhe@gmail.com/ > Would it be okay to proceed with the simpler in-place fix for > GP_TIMER_BASE declaration instead? I'm fine with v4. -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-04-01 6:55 ` Sakari Ailus @ 2026-04-01 16:02 ` Anushka B 2026-04-16 11:22 ` Sakari Ailus 0 siblings, 1 reply; 9+ messages in thread From: Anushka B @ 2026-04-01 16:02 UTC (permalink / raw) To: Sakari Ailus Cc: andriy.shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, Daniel Baluta, Dhruva gole, m-chawdhry, Simona Toaca Hi Sakari, On Wed, Apr 1, 2026 at 12:25 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > On Mon, Mar 30, 2026 at 07:02:21AM +0530, Anushka B wrote: > > > There's a lot to cleanup here, but what should be done is roughly: > > > > > > - Make these constants macros (with IPU2_ or ATOMISP2_ prefix?) and move > > > them into a separate header (perhaps with register definitions?). > > > > > > - Remove my_env and make struct device (or maybe struct atomisp_device?) as > > > a parameter for register access functions. > > > > > > This may get a bit complicated due to the amount of cleanup needed so > > > having the hardware for testing would be rather essential. > > As a student, I don't currently have access to the hardware needed to > > do the bigger refactor safely. However I'm happy to fix the comment > > formatting and language. > > In earlier versions, I had merged the split declaration of > > GP_TIMER_BASE in place in system_local.c before later moving it to > > gp_timer.c. > > Link: https://lore.kernel.org/linux-media/20260325132434.55775-1-anushkabadhe@gmail.com/ > > Would it be okay to proceed with the simpler in-place fix for > > GP_TIMER_BASE declaration instead? > > I'm fine with v4. Should I send a v7 similar to v4, where I merged the split declaration of GP_TIMER_BASE, and a separate patch for the comment fixes? Thanks, Anushka ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c 2026-04-01 16:02 ` Anushka B @ 2026-04-16 11:22 ` Sakari Ailus 0 siblings, 0 replies; 9+ messages in thread From: Sakari Ailus @ 2026-04-16 11:22 UTC (permalink / raw) To: Anushka B Cc: Sakari Ailus, andriy.shevchenko, andy, gregkh, hansg, linux-kernel, linux-media, linux-staging, mchehab, Daniel Baluta, Dhruva gole, m-chawdhry, Simona Toaca Hi Anushka, On Wed, Apr 01, 2026 at 09:32:29PM +0530, Anushka B wrote: > > > Would it be okay to proceed with the simpler in-place fix for > > > GP_TIMER_BASE declaration instead? > > > > I'm fine with v4. > > Should I send a v7 similar to v4, where I merged the split declaration > of GP_TIMER_BASE, and a separate patch for the comment fixes? Yes, please. You can find the currently applied patches in my atomisp branch here: <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=atomisp>. -- Regards, Sakari Ailus ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-04-16 11:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-03-27 3:11 [PATCH v6] staging: media: atomisp: fix GP_TIMER_BASE scope in gp_timer.c Anushka Badhe 2026-03-27 9:43 ` Andy Shevchenko 2026-03-30 1:19 ` Anushka B 2026-03-30 8:47 ` Andy Shevchenko 2026-03-27 11:24 ` Sakari Ailus 2026-03-30 1:32 ` Anushka B 2026-04-01 6:55 ` Sakari Ailus 2026-04-01 16:02 ` Anushka B 2026-04-16 11:22 ` Sakari Ailus
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®