diff mbox series

[v3,4/4] ARM: dts: marvell: Indicate USB activity on x530

Message ID 20240301014203.2033844-5-chris.packham@alliedtelesis.co.nz
State New
Headers show
Series auxdisplay: 7 segment LED display | expand

Commit Message

Chris Packham March 1, 2024, 1:42 a.m. UTC
Use the dot on the 7-segment LED block to indicate USB access on the
x530.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - None
    Changes in v2:
    - New

 arch/arm/boot/dts/marvell/armada-385-atl-x530.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Andy Shevchenko March 1, 2024, 6:24 p.m. UTC | #1
On Fri, Mar 01, 2024 at 02:42:03PM +1300, Chris Packham wrote:
> Use the dot on the 7-segment LED block to indicate USB access on the
> x530.

As I said, I'm not going to apply this even with Acks.

The problem here as I see it is the future decision on how DP should
behave like. If you put this into DT, we will to support this to the end
of the platform.

So, drop this from the next version. You may try afterwards to apply it via
different routes (will be not my problem :-).
Geert Uytterhoeven March 3, 2024, 9:48 a.m. UTC | #2
Hi Andy,

On Fri, Mar 1, 2024 at 7:24 PM Andy Shevchenko <andy@kernel.org> wrote:
> On Fri, Mar 01, 2024 at 02:42:03PM +1300, Chris Packham wrote:
> > Use the dot on the 7-segment LED block to indicate USB access on the
> > x530.
>
> As I said, I'm not going to apply this even with Acks.

I guess you should not apply any of the dts patches to the
auxdisplay tree anyway?

> The problem here as I see it is the future decision on how DP should
> behave like.  If you put this into DT, we will to support this to the end
> of the platform.

As there exist 7-seg displays (and wirings) with and without DP,
the 7-seg driver and DT bindings should handle both cases.  How to
wire/use the DP LED is up to the hardware designer / DTS writer.

I agree it's a thin boundary between hardware description and software
policy, though.  Is that your main concern?

> So, drop this from the next version. You may try afterwards to apply it via
> different routes (will be not my problem :-).

Exactly ;-)

Gr{oetje,eeting}s,

                        Geert
Chris Packham March 3, 2024, 8:11 p.m. UTC | #3
On 3/03/24 22:48, Geert Uytterhoeven wrote:
> Hi Andy,
>
> On Fri, Mar 1, 2024 at 7:24 PM Andy Shevchenko <andy@kernel.org> wrote:
>> On Fri, Mar 01, 2024 at 02:42:03PM +1300, Chris Packham wrote:
>>> Use the dot on the 7-segment LED block to indicate USB access on the
>>> x530.
>> As I said, I'm not going to apply this even with Acks.

I'll drop this one for the next round.

> I guess you should not apply any of the dts patches to the
> auxdisplay tree anyway?

That's OK by me. I've just been including them so there is an in-tree 
user of the driver. It also shows how I've been testing things.

I can send them via the ARM maintainers once the dust settles on the 
first two patches.

>
>> The problem here as I see it is the future decision on how DP should
>> behave like.  If you put this into DT, we will to support this to the end
>> of the platform.
> As there exist 7-seg displays (and wirings) with and without DP,
> the 7-seg driver and DT bindings should handle both cases.  How to
> wire/use the DP LED is up to the hardware designer / DTS writer.
>
> I agree it's a thin boundary between hardware description and software
> policy, though.  Is that your main concern?

In this specific case I'd justify the (ab)use of the DP LED on this 
product as an optimization so we don't have to find board space for a 
separate LED to indicate USB activity.

I do have an idea for handling the DP for the more general case. 
Basically if 8 segment GPIOs are supplied we can use a slightly 
different segment map (map7plus1?) so that characters that are better 
represented as dots use that instead.
Andy Shevchenko March 3, 2024, 8:43 p.m. UTC | #4
On Sun, Mar 3, 2024 at 11:48 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, Mar 1, 2024 at 7:24 PM Andy Shevchenko <andy@kernel.org> wrote:
> > On Fri, Mar 01, 2024 at 02:42:03PM +1300, Chris Packham wrote:
> > > Use the dot on the 7-segment LED block to indicate USB access on the
> > > x530.
> >
> > As I said, I'm not going to apply this even with Acks.
>
> I guess you should not apply any of the dts patches to the
> auxdisplay tree anyway?

I think it depends. If we got maintainers' Acks, etc, why not? If DT
maintainers think otherwise, then no, we shouldn't.

> > The problem here as I see it is the future decision on how DP should
> > behave like.  If you put this into DT, we will to support this to the end
> > of the platform.
>
> As there exist 7-seg displays (and wirings) with and without DP,
> the 7-seg driver and DT bindings should handle both cases.  How to
> wire/use the DP LED is up to the hardware designer / DTS writer.

Right. But my personal statistics for now is: 100% has DP (out of
about a dozen of different chip + LED combinations). What's yours?

> I agree it's a thin boundary between hardware description and software
> policy, though.  Is that your main concern?

I believe so. Because if we mark DP for use for something else, it
makes it much harder to re-use it as dot/comma later on.
Geert Uytterhoeven March 4, 2024, 9:57 a.m. UTC | #5
Hi Andy,

On Sun, Mar 3, 2024 at 9:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Mar 3, 2024 at 11:48 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Mar 1, 2024 at 7:24 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > The problem here as I see it is the future decision on how DP should
> > > behave like.  If you put this into DT, we will to support this to the end
> > > of the platform.
> >
> > As there exist 7-seg displays (and wirings) with and without DP,
> > the 7-seg driver and DT bindings should handle both cases.  How to
> > wire/use the DP LED is up to the hardware designer / DTS writer.
>
> Right. But my personal statistics for now is: 100% has DP (out of
> about a dozen of different chip + LED combinations). What's yours?

It's indeed hard to find contemporary 7-segment LED assemblies that
lack the DP.  But they do exist[1].  There's also no guarantee that the
DP is wired.
And don't forget custom or home-built assemblies using discrete LEDs,
especially for huge displays (e.g. using one LED-strip per segment).
So IMHO it would be a bad idea to make the DP mandatory.

[1] https://www.alibaba.com/product-detail/CC-CA-188-led-display-0_60626228913.html

Gr{oetje,eeting}s,

                        Geert
Andy Shevchenko March 4, 2024, 6:17 p.m. UTC | #6
On Mon, Mar 4, 2024 at 11:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sun, Mar 3, 2024 at 9:43 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:

...

> So IMHO it would be a bad idea to make the DP mandatory.

But I'm not talking about making it mandatory, I'm talking about the
DP to be used as DP when it _is_ present and wired. If current
platform wants to use DP for something else, I'm pretty much worried
that this is the right thing to do.
Geert Uytterhoeven March 4, 2024, 7:01 p.m. UTC | #7
Hi Andy,

On Mon, Mar 4, 2024 at 7:17 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Mar 4, 2024 at 11:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Sun, Mar 3, 2024 at 9:43 PM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
>
> ...
>
> > So IMHO it would be a bad idea to make the DP mandatory.
>
> But I'm not talking about making it mandatory, I'm talking about the

OK.

> DP to be used as DP when it _is_ present and wired. If current
> platform wants to use DP for something else, I'm pretty much worried
> that this is the right thing to do.

There is not much we can do about that. People can already model
such displays as individual LEDs, too.
And in some sense, the auxdisplay/linedisp driver for
"generic-gpio-7seg" imposes a policy, too.
What if people want to e.g. use 4 7-seg displays to show a continuously
running snake?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Andy Shevchenko March 4, 2024, 7:16 p.m. UTC | #8
On Mon, Mar 04, 2024 at 08:01:58PM +0100, Geert Uytterhoeven wrote:
> On Mon, Mar 4, 2024 at 7:17 PM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Mar 4, 2024 at 11:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Sun, Mar 3, 2024 at 9:43 PM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:

...

> > > So IMHO it would be a bad idea to make the DP mandatory.
> >
> > But I'm not talking about making it mandatory, I'm talking about the
> 
> OK.
> 
> > DP to be used as DP when it _is_ present and wired. If current
> > platform wants to use DP for something else, I'm pretty much worried
> > that this is the right thing to do.
> 
> There is not much we can do about that. People can already model
> such displays as individual LEDs, too.
> And in some sense, the auxdisplay/linedisp driver for
> "generic-gpio-7seg" imposes a policy, too.

Does it? It's exactly targeting very specific HW configuration. The only
question here is DP.

> What if people want to e.g. use 4 7-seg displays to show a continuously
> running snake?

We have an ABI to update a "character" mapping, so it's possible to do, but
it is not a main purpose of line display library.

Free running 7-segment display does probably belong to LED framework in that
sense (as just represents a 7 LEDs that user configured in a specific way in
the physical world). In such case it's just the 7 LEDs on a single PCB.

If you consider these limits as "policy", okay, but it's _hardware driven_
one, and not software.
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts b/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
index 60c2abf572b6..a53dd17d11b4 100644
--- a/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
+++ b/arch/arm/boot/dts/marvell/armada-385-atl-x530.dts
@@ -54,6 +54,15 @@  &led_7seg_gpio 4 GPIO_ACTIVE_LOW
 				 &led_7seg_gpio 5 GPIO_ACTIVE_LOW
 				 &led_7seg_gpio 6 GPIO_ACTIVE_LOW>;
 	};
+
+	leds {
+		compatible = "gpio-leds";
+		led-0 {
+			label = "usb";
+			gpios =  <&led_7seg_gpio 7 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "usb-host";
+		};
+	};
 };
 
 &pciec {