2023-11-17 12:13:17

by Lee Jones

[permalink] [raw]
Subject: Re: [Patch v8 6/6] leds: ledtrig-tty: add additional line state evaluation

On Thu, 09 Nov 2023, [email protected] wrote:

> Florian Eckert schreef op 2023-11-09 09:50:
> > The serial tty interface also supports additional input signals, that
> > can also be evaluated within this trigger. This change is adding the
> > following additional input sources, which could be controlled
> > via the '/sys/class/<leds>/' sysfs interface.
> >
> > Explanation:
> > DCE = Data Communication Equipment (Modem)
> > DTE = Data Terminal Equipment (Computer)
> >
> > - cts:
> > DCE is ready to accept data from the DTE (CTS = Clear To Send). If
> > the line state is detected, the LED is switched on.
> > If set to 0 (default), the LED will not evaluate CTS.
> > If set to 1, the LED will evaluate CTS.
> >
> > - dsr:
> > DCE is ready to receive and send data (DSR = Data Set Ready). If the
> > line state is detected, the LED is switched on.
> > If set to 0 (default), the LED will not evaluate DSR.
> > If set to 1, the LED will evaluate DSR.
> >
> > - dcd:
> > DTE is receiving a carrier from the DCE (DCD = Data Carrier Detect).
> > If the line state is detected, the LED is switched on.
> > If set to 0 (default), the LED will not evaluate DCD.
> > If set to 1, the LED will evaluate DCD.
> >
> > - rng:
> > DCE has detected an incoming ring signal on the telephone line
> > (RNG = Ring Indicator). If the line state is detected, the LED is
> > switched on.
> > If set to 0 (default), the LED will not evaluate RNG.
> > If set to 1, the LED will evaluate RNG.
> >
> > Also add an invert flag on LED blink, so that the LED blinks in the
> > correct order.
> >
> > * If one off the new enabled input signals are evaluatet as 'enabled',
> > and data are transmitted, then the LED should first blink 'off' and
> > then 'on' (invert).
> > * If all the new enabled input signals are evaluatet as 'disabled',
> > and data are transmitted, then the LED should first blink 'on' and
> > then 'off'.
> >
> > Signed-off-by: Florian Eckert <[email protected]>
> > ---
> > .../ABI/testing/sysfs-class-led-trigger-tty | 40 ++++++++++
> > drivers/leds/trigger/ledtrig-tty.c | 77 ++++++++++++++++++-
> > 2 files changed, 116 insertions(+), 1 deletion(-)

[...]

> Reviewed-by: Maarten Brock <[email protected]>

Please snip your replies.

--
Lee Jones [李琼斯]


2023-11-20 07:22:07

by Florian Eckert

[permalink] [raw]
Subject: Re: [Patch v8 6/6] leds: ledtrig-tty: add additional line state evaluation



On 2023-11-17 13:12, Lee Jones wrote:
> On Thu, 09 Nov 2023, [email protected] wrote:
>
>> Florian Eckert schreef op 2023-11-09 09:50:
>> > The serial tty interface also supports additional input signals, that
>> > can also be evaluated within this trigger. This change is adding the
>> > following additional input sources, which could be controlled
>> > via the '/sys/class/<leds>/' sysfs interface.
>> >
>> > Explanation:
>> > DCE = Data Communication Equipment (Modem)
>> > DTE = Data Terminal Equipment (Computer)
>> >
>> > - cts:
>> > DCE is ready to accept data from the DTE (CTS = Clear To Send). If
>> > the line state is detected, the LED is switched on.
>> > If set to 0 (default), the LED will not evaluate CTS.
>> > If set to 1, the LED will evaluate CTS.
>> >
>> > - dsr:
>> > DCE is ready to receive and send data (DSR = Data Set Ready). If the
>> > line state is detected, the LED is switched on.
>> > If set to 0 (default), the LED will not evaluate DSR.
>> > If set to 1, the LED will evaluate DSR.
>> >
>> > - dcd:
>> > DTE is receiving a carrier from the DCE (DCD = Data Carrier Detect).
>> > If the line state is detected, the LED is switched on.
>> > If set to 0 (default), the LED will not evaluate DCD.
>> > If set to 1, the LED will evaluate DCD.
>> >
>> > - rng:
>> > DCE has detected an incoming ring signal on the telephone line
>> > (RNG = Ring Indicator). If the line state is detected, the LED is
>> > switched on.
>> > If set to 0 (default), the LED will not evaluate RNG.
>> > If set to 1, the LED will evaluate RNG.
>> >
>> > Also add an invert flag on LED blink, so that the LED blinks in the
>> > correct order.
>> >
>> > * If one off the new enabled input signals are evaluatet as 'enabled',
>> > and data are transmitted, then the LED should first blink 'off' and
>> > then 'on' (invert).
>> > * If all the new enabled input signals are evaluatet as 'disabled',
>> > and data are transmitted, then the LED should first blink 'on' and
>> > then 'off'.
>> >
>> > Signed-off-by: Florian Eckert <[email protected]>
>> > ---
>> > .../ABI/testing/sysfs-class-led-trigger-tty | 40 ++++++++++
>> > drivers/leds/trigger/ledtrig-tty.c | 77 ++++++++++++++++++-
>> > 2 files changed, 116 insertions(+), 1 deletion(-)
>
> [...]
>
>> Reviewed-by: Maarten Brock <[email protected]>
>
> Please snip your replies.

Is there anything I can do? Or do I have to do something? Please give me
more detailed instructions

2023-11-21 15:24:00

by Lee Jones

[permalink] [raw]
Subject: Re: [Patch v8 6/6] leds: ledtrig-tty: add additional line state evaluation

On Mon, 20 Nov 2023, Florian Eckert wrote:

>
>
> On 2023-11-17 13:12, Lee Jones wrote:
> > On Thu, 09 Nov 2023, [email protected] wrote:
> >
> > > Florian Eckert schreef op 2023-11-09 09:50:
> > > > The serial tty interface also supports additional input signals, that
> > > > can also be evaluated within this trigger. This change is adding the
> > > > following additional input sources, which could be controlled
> > > > via the '/sys/class/<leds>/' sysfs interface.
> > > >
> > > > Explanation:
> > > > DCE = Data Communication Equipment (Modem)
> > > > DTE = Data Terminal Equipment (Computer)
> > > >
> > > > - cts:
> > > > DCE is ready to accept data from the DTE (CTS = Clear To Send). If
> > > > the line state is detected, the LED is switched on.
> > > > If set to 0 (default), the LED will not evaluate CTS.
> > > > If set to 1, the LED will evaluate CTS.
> > > >
> > > > - dsr:
> > > > DCE is ready to receive and send data (DSR = Data Set Ready). If the
> > > > line state is detected, the LED is switched on.
> > > > If set to 0 (default), the LED will not evaluate DSR.
> > > > If set to 1, the LED will evaluate DSR.
> > > >
> > > > - dcd:
> > > > DTE is receiving a carrier from the DCE (DCD = Data Carrier Detect).
> > > > If the line state is detected, the LED is switched on.
> > > > If set to 0 (default), the LED will not evaluate DCD.
> > > > If set to 1, the LED will evaluate DCD.
> > > >
> > > > - rng:
> > > > DCE has detected an incoming ring signal on the telephone line
> > > > (RNG = Ring Indicator). If the line state is detected, the LED is
> > > > switched on.
> > > > If set to 0 (default), the LED will not evaluate RNG.
> > > > If set to 1, the LED will evaluate RNG.
> > > >
> > > > Also add an invert flag on LED blink, so that the LED blinks in the
> > > > correct order.
> > > >
> > > > * If one off the new enabled input signals are evaluatet as 'enabled',
> > > > and data are transmitted, then the LED should first blink 'off' and
> > > > then 'on' (invert).
> > > > * If all the new enabled input signals are evaluatet as 'disabled',
> > > > and data are transmitted, then the LED should first blink 'on' and
> > > > then 'off'.
> > > >
> > > > Signed-off-by: Florian Eckert <[email protected]>
> > > > ---
> > > > .../ABI/testing/sysfs-class-led-trigger-tty | 40 ++++++++++
> > > > drivers/leds/trigger/ledtrig-tty.c | 77 ++++++++++++++++++-
> > > > 2 files changed, 116 insertions(+), 1 deletion(-)
> >
> > [...]
> >
> > > Reviewed-by: Maarten Brock <[email protected]>
> >
> > Please snip your replies.
>
> Is there anything I can do? Or do I have to do something? Please give me
> more detailed instructions

That instruction wasn't for you.

This patch is still on my INCOMING list.

Do you have Greg's blessing yet?

--
Lee Jones [李琼斯]

2023-11-22 09:53:01

by Florian Eckert

[permalink] [raw]
Subject: Re: [Patch v8 6/6] leds: ledtrig-tty: add additional line state evaluation



On 2023-11-21 16:23, Lee Jones wrote:
> On Mon, 20 Nov 2023, Florian Eckert wrote:
>
>>
>>
>> On 2023-11-17 13:12, Lee Jones wrote:
>> > On Thu, 09 Nov 2023, [email protected] wrote:
>> >
>> > > Florian Eckert schreef op 2023-11-09 09:50:
>> > > > The serial tty interface also supports additional input signals, that
>> > > > can also be evaluated within this trigger. This change is adding the
>> > > > following additional input sources, which could be controlled
>> > > > via the '/sys/class/<leds>/' sysfs interface.
>> > > >
>> > > > Explanation:
>> > > > DCE = Data Communication Equipment (Modem)
>> > > > DTE = Data Terminal Equipment (Computer)
>> > > >
>> > > > - cts:
>> > > > DCE is ready to accept data from the DTE (CTS = Clear To Send). If
>> > > > the line state is detected, the LED is switched on.
>> > > > If set to 0 (default), the LED will not evaluate CTS.
>> > > > If set to 1, the LED will evaluate CTS.
>> > > >
>> > > > - dsr:
>> > > > DCE is ready to receive and send data (DSR = Data Set Ready). If the
>> > > > line state is detected, the LED is switched on.
>> > > > If set to 0 (default), the LED will not evaluate DSR.
>> > > > If set to 1, the LED will evaluate DSR.
>> > > >
>> > > > - dcd:
>> > > > DTE is receiving a carrier from the DCE (DCD = Data Carrier Detect).
>> > > > If the line state is detected, the LED is switched on.
>> > > > If set to 0 (default), the LED will not evaluate DCD.
>> > > > If set to 1, the LED will evaluate DCD.
>> > > >
>> > > > - rng:
>> > > > DCE has detected an incoming ring signal on the telephone line
>> > > > (RNG = Ring Indicator). If the line state is detected, the LED is
>> > > > switched on.
>> > > > If set to 0 (default), the LED will not evaluate RNG.
>> > > > If set to 1, the LED will evaluate RNG.
>> > > >
>> > > > Also add an invert flag on LED blink, so that the LED blinks in the
>> > > > correct order.
>> > > >
>> > > > * If one off the new enabled input signals are evaluatet as 'enabled',
>> > > > and data are transmitted, then the LED should first blink 'off' and
>> > > > then 'on' (invert).
>> > > > * If all the new enabled input signals are evaluatet as 'disabled',
>> > > > and data are transmitted, then the LED should first blink 'on' and
>> > > > then 'off'.
>> > > >
>> > > > Signed-off-by: Florian Eckert <[email protected]>
>> > > > ---
>> > > > .../ABI/testing/sysfs-class-led-trigger-tty | 40 ++++++++++
>> > > > drivers/leds/trigger/ledtrig-tty.c | 77 ++++++++++++++++++-
>> > > > 2 files changed, 116 insertions(+), 1 deletion(-)
>> >
>> > [...]
>> >
>> > > Reviewed-by: Maarten Brock <[email protected]>
>> >
>> > Please snip your replies.
>>
>> Is there anything I can do? Or do I have to do something? Please give
>> me
>> more detailed instructions
>
> That instruction wasn't for you.
>
> This patch is still on my INCOMING list.
>
> Do you have Greg's blessing yet?

The summary of my v8:

The changes for the tty layer on v5 in 'drivers/tty/tty_io.c' got an
'Acked-by: Greg Kroah-Hartman' [1].
I have always added his 'Acked-by' to the following patch series.
And I did not made changes to this. So I think I have his blessing
for this changes in his maintained tty layer.

The Memory leak patch I send during v7 [2] got a comment
from Greg, that I have to send this also to
'[email protected]'
So this should go into the stable branch [3]. This got an
'Reviewed-by: Uwe Kleine-König' [4]. I add this to v8 [5].
So far I don't know if this has already been merged into
the master and then backported into the stable branches?.

For the changes in the ledtrig-tty driver I am still waiting for an
complete 'ACK' or 'NOK' whether I should change something.
I have added all of Greg's requested changes in v5 [6]:
* split this series
* Add the requested change
* Switch the driver to use completion for 'sysfs'

As I understand it, he handed over the review to the LED subsystem team
[7].

I then added a few more changes that came from Maarten in v7 [8].
I got his 'Reviewed-by: Maarten Brock' for v8 on patch 6/6.

The patches 4/6 and 5/6 of the v8 still waiting for review?

Best regards

Florian

[1]
https://lore.kernel.org/linux-leds/2023102327-rename-kosher-bf03@gregkh/#t
[2]
https://lore.kernel.org/linux-leds/2023110629-scenic-rounding-905f@gregkh/
[3]
https://lore.kernel.org/linux-leds/[email protected]/
[4]
https://lore.kernel.org/linux-leds/[email protected]/
[5]
https://lore.kernel.org/linux-leds/[email protected]/T/#m1f0c4680749812f1a933667128f73995efe66bca
[6]
https://lore.kernel.org/linux-leds/2023102341-jogger-matching-dded@gregkh/
[7]
https://lore.kernel.org/linux-leds/2023102333-skewer-reclining-8d04@gregkh/
[8]
https://lore.kernel.org/linux-leds/[email protected]/
[9]
https://lore.kernel.org/linux-leds/[email protected]/

2023-11-22 11:29:48

by Lee Jones

[permalink] [raw]
Subject: Re: [Patch v8 6/6] leds: ledtrig-tty: add additional line state evaluation

On Wed, 22 Nov 2023, Florian Eckert wrote:

>
>
> On 2023-11-21 16:23, Lee Jones wrote:
> > On Mon, 20 Nov 2023, Florian Eckert wrote:
> >
> > >
> > >
> > > On 2023-11-17 13:12, Lee Jones wrote:
> > > > On Thu, 09 Nov 2023, [email protected] wrote:
> > > >
> > > > > Florian Eckert schreef op 2023-11-09 09:50:
> > > > > > The serial tty interface also supports additional input signals, that
> > > > > > can also be evaluated within this trigger. This change is adding the
> > > > > > following additional input sources, which could be controlled
> > > > > > via the '/sys/class/<leds>/' sysfs interface.
> > > > > >
> > > > > > Explanation:
> > > > > > DCE = Data Communication Equipment (Modem)
> > > > > > DTE = Data Terminal Equipment (Computer)
> > > > > >
> > > > > > - cts:
> > > > > > DCE is ready to accept data from the DTE (CTS = Clear To Send). If
> > > > > > the line state is detected, the LED is switched on.
> > > > > > If set to 0 (default), the LED will not evaluate CTS.
> > > > > > If set to 1, the LED will evaluate CTS.
> > > > > >
> > > > > > - dsr:
> > > > > > DCE is ready to receive and send data (DSR = Data Set Ready). If the
> > > > > > line state is detected, the LED is switched on.
> > > > > > If set to 0 (default), the LED will not evaluate DSR.
> > > > > > If set to 1, the LED will evaluate DSR.
> > > > > >
> > > > > > - dcd:
> > > > > > DTE is receiving a carrier from the DCE (DCD = Data Carrier Detect).
> > > > > > If the line state is detected, the LED is switched on.
> > > > > > If set to 0 (default), the LED will not evaluate DCD.
> > > > > > If set to 1, the LED will evaluate DCD.
> > > > > >
> > > > > > - rng:
> > > > > > DCE has detected an incoming ring signal on the telephone line
> > > > > > (RNG = Ring Indicator). If the line state is detected, the LED is
> > > > > > switched on.
> > > > > > If set to 0 (default), the LED will not evaluate RNG.
> > > > > > If set to 1, the LED will evaluate RNG.
> > > > > >
> > > > > > Also add an invert flag on LED blink, so that the LED blinks in the
> > > > > > correct order.
> > > > > >
> > > > > > * If one off the new enabled input signals are evaluatet as 'enabled',
> > > > > > and data are transmitted, then the LED should first blink 'off' and
> > > > > > then 'on' (invert).
> > > > > > * If all the new enabled input signals are evaluatet as 'disabled',
> > > > > > and data are transmitted, then the LED should first blink 'on' and
> > > > > > then 'off'.
> > > > > >
> > > > > > Signed-off-by: Florian Eckert <[email protected]>
> > > > > > ---
> > > > > > .../ABI/testing/sysfs-class-led-trigger-tty | 40 ++++++++++
> > > > > > drivers/leds/trigger/ledtrig-tty.c | 77 ++++++++++++++++++-
> > > > > > 2 files changed, 116 insertions(+), 1 deletion(-)
> > > >
> > > > [...]
> > > >
> > > > > Reviewed-by: Maarten Brock <[email protected]>
> > > >
> > > > Please snip your replies.
> > >
> > > Is there anything I can do? Or do I have to do something? Please
> > > give me
> > > more detailed instructions
> >
> > That instruction wasn't for you.
> >
> > This patch is still on my INCOMING list.
> >
> > Do you have Greg's blessing yet?
>
> The summary of my v8:
>
> The changes for the tty layer on v5 in 'drivers/tty/tty_io.c' got an
> 'Acked-by: Greg Kroah-Hartman' [1].
> I have always added his 'Acked-by' to the following patch series.
> And I did not made changes to this. So I think I have his blessing
> for this changes in his maintained tty layer.
>
> The Memory leak patch I send during v7 [2] got a comment
> from Greg, that I have to send this also to '[email protected]'
> So this should go into the stable branch [3]. This got an
> 'Reviewed-by: Uwe Kleine-König' [4]. I add this to v8 [5].
> So far I don't know if this has already been merged into
> the master and then backported into the stable branches?.
>
> For the changes in the ledtrig-tty driver I am still waiting for an
> complete 'ACK' or 'NOK' whether I should change something.
> I have added all of Greg's requested changes in v5 [6]:
> * split this series
> * Add the requested change
> * Switch the driver to use completion for 'sysfs'
>
> As I understand it, he handed over the review to the LED subsystem team [7].
>
> I then added a few more changes that came from Maarten in v7 [8].
> I got his 'Reviewed-by: Maarten Brock' for v8 on patch 6/6.
>
> The patches 4/6 and 5/6 of the v8 still waiting for review?

Thanks for the update Florian.

Sounds like you're waiting on me and/or Pavel.

You're in the pile. I'll get around to you shortly.

> [1]
> https://lore.kernel.org/linux-leds/2023102327-rename-kosher-bf03@gregkh/#t
> [2]
> https://lore.kernel.org/linux-leds/2023110629-scenic-rounding-905f@gregkh/
> [3]
> https://lore.kernel.org/linux-leds/[email protected]/
> [4] https://lore.kernel.org/linux-leds/[email protected]/
> [5] https://lore.kernel.org/linux-leds/[email protected]/T/#m1f0c4680749812f1a933667128f73995efe66bca
> [6]
> https://lore.kernel.org/linux-leds/2023102341-jogger-matching-dded@gregkh/
> [7]
> https://lore.kernel.org/linux-leds/2023102333-skewer-reclining-8d04@gregkh/
> [8] https://lore.kernel.org/linux-leds/[email protected]/
> [9] https://lore.kernel.org/linux-leds/[email protected]/

--
Lee Jones [李琼斯]