| Message ID | 20230928132632.200263-4-fe@dev.tdt.de |
|---|---|
| State | New |
| Headers | show |
| Series | ledtrig-tty: add new state evaluation | expand |
On 28. 09. 23, 15:26, Florian Eckert wrote: > The Intel build robot has complained about this. How exactly? > Hence move the commit > of the variable definition to the beginning of the function. > > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- > drivers/leds/trigger/ledtrig-tty.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c > index 8ae0d2d284af..1c6fadf0b856 100644 > --- a/drivers/leds/trigger/ledtrig-tty.c > +++ b/drivers/leds/trigger/ledtrig-tty.c > @@ -82,6 +82,7 @@ static void ledtrig_tty_work(struct work_struct *work) > { > struct ledtrig_tty_data *trigger_data = > container_of(work, struct ledtrig_tty_data, dwork.work); > + unsigned long interval = LEDTRIG_TTY_INTERVAL; > struct serial_icounter_struct icount; > int ret; > > @@ -124,8 +125,6 @@ static void ledtrig_tty_work(struct work_struct *work) > > if (icount.rx != trigger_data->rx || > icount.tx != trigger_data->tx) { > - unsigned long interval = LEDTRIG_TTY_INTERVAL; It's in a block, so what's the matter? > led_blink_set_oneshot(trigger_data->led_cdev, &interval, > &interval, 0); >
On Thu, 28 Sep 2023, Florian Eckert wrote: > The Intel build robot has complained about this. Hence move the commit > of the variable definition to the beginning of the function. Please copy the robot's error message into the commit message. > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Florian Eckert <fe@dev.tdt.de> > --- > drivers/leds/trigger/ledtrig-tty.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c > index 8ae0d2d284af..1c6fadf0b856 100644 > --- a/drivers/leds/trigger/ledtrig-tty.c > +++ b/drivers/leds/trigger/ledtrig-tty.c > @@ -82,6 +82,7 @@ static void ledtrig_tty_work(struct work_struct *work) > { > struct ledtrig_tty_data *trigger_data = > container_of(work, struct ledtrig_tty_data, dwork.work); > + unsigned long interval = LEDTRIG_TTY_INTERVAL; > struct serial_icounter_struct icount; > int ret; > > @@ -124,8 +125,6 @@ static void ledtrig_tty_work(struct work_struct *work) > > if (icount.rx != trigger_data->rx || > icount.tx != trigger_data->tx) { > - unsigned long interval = LEDTRIG_TTY_INTERVAL; > - > led_blink_set_oneshot(trigger_data->led_cdev, &interval, > &interval, 0); > > -- > 2.30.2 >
On 02. 10. 23, 16:05, Lee Jones wrote: > On Thu, 28 Sep 2023, Florian Eckert wrote: > >> The Intel build robot has complained about this. Hence move the commit >> of the variable definition to the beginning of the function. > > Please copy the robot's error message into the commit message. Ah, lkp, then also the Closes: line as it suggests. >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Florian Eckert <fe@dev.tdt.de> >> --- >> drivers/leds/trigger/ledtrig-tty.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c >> index 8ae0d2d284af..1c6fadf0b856 100644 >> --- a/drivers/leds/trigger/ledtrig-tty.c >> +++ b/drivers/leds/trigger/ledtrig-tty.c >> @@ -82,6 +82,7 @@ static void ledtrig_tty_work(struct work_struct *work) >> { >> struct ledtrig_tty_data *trigger_data = >> container_of(work, struct ledtrig_tty_data, dwork.work); >> + unsigned long interval = LEDTRIG_TTY_INTERVAL; >> struct serial_icounter_struct icount; >> int ret; >> >> @@ -124,8 +125,6 @@ static void ledtrig_tty_work(struct work_struct *work) >> >> if (icount.rx != trigger_data->rx || >> icount.tx != trigger_data->tx) { >> - unsigned long interval = LEDTRIG_TTY_INTERVAL; >> - >> led_blink_set_oneshot(trigger_data->led_cdev, &interval, >> &interval, 0); >> >> -- >> 2.30.2 >> >
On 2023-10-03 07:00, Jiri Slaby wrote: > On 02. 10. 23, 16:05, Lee Jones wrote: >> On Thu, 28 Sep 2023, Florian Eckert wrote: >> >>> The Intel build robot has complained about this. Hence move the >>> commit >>> of the variable definition to the beginning of the function. >> Please copy the robot's error message into the commit message. For a v3 patch-set I will add the error message from build robot. Build robot output of my v1 change: https://lore.kernel.org/linux-leds/20230926093607.59536-1-fe@dev.tdt.de/T/#m777371c5de8fadc505a833139b8ae69ac7fa8dab I decided to move the variable definition with a separate commit to the top of the function, to make the build robot happy. After that I made my changes for v2 to the ledtrig-tty to add the feature. > Ah, lkp, then also the Closes: line as it suggests. Sorry I do not understand your statement >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: Florian Eckert <fe@dev.tdt.de> >>> --- >>> drivers/leds/trigger/ledtrig-tty.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/drivers/leds/trigger/ledtrig-tty.c >>> b/drivers/leds/trigger/ledtrig-tty.c >>> index 8ae0d2d284af..1c6fadf0b856 100644 >>> --- a/drivers/leds/trigger/ledtrig-tty.c >>> +++ b/drivers/leds/trigger/ledtrig-tty.c >>> @@ -82,6 +82,7 @@ static void ledtrig_tty_work(struct work_struct >>> *work) >>> { >>> struct ledtrig_tty_data *trigger_data = >>> container_of(work, struct ledtrig_tty_data, dwork.work); >>> + unsigned long interval = LEDTRIG_TTY_INTERVAL; >>> struct serial_icounter_struct icount; >>> int ret; >>> @@ -124,8 +125,6 @@ static void ledtrig_tty_work(struct work_struct >>> *work) >>> if (icount.rx != trigger_data->rx || >>> icount.tx != trigger_data->tx) { >>> - unsigned long interval = LEDTRIG_TTY_INTERVAL; >>> - >>> led_blink_set_oneshot(trigger_data->led_cdev, &interval, >>> &interval, 0); >>> -- 2.30.2 >>> >>
On 04. 10. 23, 8:37, Florian Eckert wrote: > On 2023-10-03 07:00, Jiri Slaby wrote: >> On 02. 10. 23, 16:05, Lee Jones wrote: >>> On Thu, 28 Sep 2023, Florian Eckert wrote: >>> >>>> The Intel build robot has complained about this. Hence move the commit >>>> of the variable definition to the beginning of the function. > >>> Please copy the robot's error message into the commit message. > > For a v3 patch-set I will add the error message from build robot. > > Build robot output of my v1 change: > https://lore.kernel.org/linux-leds/20230926093607.59536-1-fe@dev.tdt.de/T/#m777371c5de8fadc505a833139b8ae69ac7fa8dab > > I decided to move the variable definition with a separate commit > to the top of the function, to make the build robot happy. After that > I made my changes for v2 to the ledtrig-tty to add the feature. > >> Ah, lkp, then also the Closes: line as it suggests. > > Sorry I do not understand your statement The link you pasted above states: ======= If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309270440.IJB24Xap-lkp@intel.com/ ======= So please follow that suggestion ;). thanks,
On Thu, 05 Oct 2023, Greg KH wrote: > On Thu, Oct 05, 2023 at 11:13:07AM +0100, Lee Jones wrote: > > On Thu, 05 Oct 2023, Greg KH wrote: > > > > > On Wed, Oct 04, 2023 at 10:36:09AM +0200, Florian Eckert wrote: > > > > > > > > > > > > > > I decided to move the variable definition with a separate commit > > > > > > to the top of the function, to make the build robot happy. After that > > > > > > I made my changes for v2 to the ledtrig-tty to add the feature. > > > > > > > > > > > > > Ah, lkp, then also the Closes: line as it suggests. > > > > > > > > > > > > Sorry I do not understand your statement > > > > > > > > > > The link you pasted above states: > > > > > ======= > > > > > If you fix the issue in a separate patch/commit (i.e. not just a new > > > > > version of > > > > > the same patch/commit), kindly add following tags > > > > > | Reported-by: kernel test robot <lkp@intel.com> > > > > > | Closes: > > > > > https://lore.kernel.org/oe-kbuild-all/202309270440.IJB24Xap-lkp@intel.com/ > > > > > ======= > > > > > > > > > > So please follow that suggestion ;). > > > > > > > > Ok, I understand, thanks will to this on a v3 patchset. > > > > I will now wait for the comments of my changes in ledtrig-tty from the led > > > > subsystem. > > > > And then I will send a new patch set with the requested changes. > > > > > > > > Sorry for the silly question. But do I have to send this patch again for a > > > > v3? > > > > https://lore.kernel.org/linux-leds/f41dc1e1-6d34-48b2-97dd-ba67df6003c6@kernel.org/T/#u > > > > It was already marked by you with a `Reviewed-by:` from you? > > > > Yes please. I will pick this up as a set once it's ready. > > > > > This series is long gone from my review queue, so a v3 will be needed at > > > the very least. > > > > Nothing for Greg to worry about here (unless you *want* to review). > > Yes, I want to ensure that the tty change is correct, last round I > didn't think it was... Sounds good, thanks.
Hello Lee, I only got reviews for the fixes and preparations for commits that change the tty subsystem, but no reaction from the maintainer of the feature I want to add to ledtrig-tty for v1 and v2 patchset. How should I proceed? Send a v3 with the the requested changes. [Patch v2 1/4]: https://lore.kernel.org/linux-leds/20230928132632.200263-1-fe@dev.tdt.de/T/#m913d3822465f35b54dfa24b1dfe4d50e61352980 Change got a 'Reviewed-by: Jiri Slaby <jirislaby@kernel.org>'. Will add this to an upcoming v3 again. [Patch v2 2/4] : https://lore.kernel.org/linux-leds/20230928132632.200263-1-fe@dev.tdt.de/T/#m7ee7618894a66fd3c89bed488a2394265a3f8df1 I missed to add the robot error message to the commit message and also missed to add the the following 'Reported-by: kernel test robot <lkp@intel.com>' and 'Closes: https://lore.kernel.org/oe-kbuild-all/202309270440.IJB24Xap-lkp@intel.com/' to the commit message. Will add this to an upcoming v3. And do not wait for the review of the following patches. https://lore.kernel.org/linux-leds/20230928132632.200263-1-fe@dev.tdt.de/T/#mc0ecb912fa0e59015ad0a9b4cb491ae9f18c1ea9 https://lore.kernel.org/linux-leds/20230928132632.200263-1-fe@dev.tdt.de/T/#mba36217323c386ecd900e188bbdf6276c3c96c91 --- Florian
diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c index 8ae0d2d284af..1c6fadf0b856 100644 --- a/drivers/leds/trigger/ledtrig-tty.c +++ b/drivers/leds/trigger/ledtrig-tty.c @@ -82,6 +82,7 @@ static void ledtrig_tty_work(struct work_struct *work) { struct ledtrig_tty_data *trigger_data = container_of(work, struct ledtrig_tty_data, dwork.work); + unsigned long interval = LEDTRIG_TTY_INTERVAL; struct serial_icounter_struct icount; int ret; @@ -124,8 +125,6 @@ static void ledtrig_tty_work(struct work_struct *work) if (icount.rx != trigger_data->rx || icount.tx != trigger_data->tx) { - unsigned long interval = LEDTRIG_TTY_INTERVAL; - led_blink_set_oneshot(trigger_data->led_cdev, &interval, &interval, 0);
The Intel build robot has complained about this. Hence move the commit of the variable definition to the beginning of the function. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Florian Eckert <fe@dev.tdt.de> --- drivers/leds/trigger/ledtrig-tty.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)