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(-)
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.2On 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
>
--
Lee Jones [李琼斯]
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
On Wed, 11 Oct 2023, Florian Eckert wrote: > 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 Yes. I've removed this from my queue. Better to resend it with the fixes. -- Lee Jones [李琼斯]
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
>>
>
--
js
suse labs
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, -- js suse labs
>> 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? -- Best regards Florian
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? This series is long gone from my review queue, so a v3 will be needed at the very least. thanks, greg k-h
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). -- Lee Jones [李琼斯]
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... thanks, greg k-h
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. -- Lee Jones [李琼斯]
On 04. 10. 23, 10:36, Florian Eckert wrote: > 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? If it is not picked up by Greg by then, I would send it with my rev-b already. thanks, -- js suse labs
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);
>
--
js
suse labs
© 2016 - 2025 Red Hat, Inc.