Linux-LEDs Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
       [not found]     ` <20190816212728.GW250418@google.com>
@ 2019-08-17 14:05       ` Pavel Machek
  2019-08-19  0:37         ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Machek @ 2019-08-17 14:05 UTC (permalink / raw)
  To: Matthias Kaehlcke, jacek.anaszewski, linux-leds, dmurphy
  Cc: David S . Miller, Rob Herring, Mark Rutland, Andrew Lunn,
	Florian Fainelli, Heiner Kallweit, netdev, devicetree,
	linux-kernel, Douglas Anderson

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

On Fri 2019-08-16 14:27:28, Matthias Kaehlcke wrote:
> On Fri, Aug 16, 2019 at 10:13:42PM +0200, Pavel Machek wrote:
> > On Tue 2019-08-13 12:11:47, Matthias Kaehlcke wrote:
> > > Add a .config_led hook which is called by the PHY core when
> > > configuration data for a PHY LED is available. Each LED can be
> > > configured to be solid 'off, solid 'on' for certain (or all)
> > > link speeds or to blink on RX/TX activity.
> > > 
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > 
> > THis really needs to go through the LED subsystem,
> 
> Sorry, I used what get_maintainers.pl threw at me, I should have
> manually cc-ed the LED list.
> 
> > and use the same userland interfaces as the rest of the system.
> 
> With the PHY maintainers we discussed to define a binding that is
> compatible with that of the LED one, to have the option to integrate
> it with the LED subsystem later. The integration itself is beyond the
> scope of this patchset.

Yes, I believe the integration is neccessary. Using same binding is
neccessary for that, but not sufficient. For example, we need
compatible trigger names, too.

So... I'd really like to see proper integration is possible before we
merge this.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
  2019-08-17 14:05       ` [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E Pavel Machek
@ 2019-08-19  0:37         ` Andrew Lunn
  2019-10-07 11:02           ` Pavel Machek
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2019-08-19  0:37 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Matthias Kaehlcke, jacek.anaszewski, linux-leds, dmurphy,
	David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
	Heiner Kallweit, netdev, devicetree, linux-kernel,
	Douglas Anderson

> Yes, I believe the integration is neccessary. Using same binding is
> neccessary for that, but not sufficient. For example, we need
> compatible trigger names, too.

Hi Pavel

Please could you explain what you mean by compatible trigger names?

> So... I'd really like to see proper integration is possible before we
> merge this.

Please let me turn that around. What do you see as being impossible at
the moment? What do we need to convince you about?

    Thanks
	Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E
  2019-08-19  0:37         ` Andrew Lunn
@ 2019-10-07 11:02           ` Pavel Machek
  0 siblings, 0 replies; 3+ messages in thread
From: Pavel Machek @ 2019-10-07 11:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Matthias Kaehlcke, jacek.anaszewski, linux-leds, dmurphy,
	David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
	Heiner Kallweit, netdev, devicetree, linux-kernel,
	Douglas Anderson

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Mon 2019-08-19 02:37:57, Andrew Lunn wrote:
> > Yes, I believe the integration is neccessary. Using same binding is
> > neccessary for that, but not sufficient. For example, we need
> > compatible trigger names, too.
> 
> Hi Pavel
> 
> Please could you explain what you mean by compatible trigger names?

Well, you attempted to put trigger names in device tree. That means
those names should work w.r.t. LED subsystem, too.

> > So... I'd really like to see proper integration is possible before we
> > merge this.
> 
> Please let me turn that around. What do you see as being impossible at
> the moment? What do we need to convince you about?

That locking requirements are compatible, that triggers you invented
can be implemented by LED subsystem, ...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190813191147.19936-1-mka@chromium.org>
     [not found] ` <20190813191147.19936-5-mka@chromium.org>
     [not found]   ` <20190816201342.GB1646@bug>
     [not found]     ` <20190816212728.GW250418@google.com>
2019-08-17 14:05       ` [PATCH v6 4/4] net: phy: realtek: Add LED configuration support for RTL8211E Pavel Machek
2019-08-19  0:37         ` Andrew Lunn
2019-10-07 11:02           ` Pavel Machek

Linux-LEDs Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-leds/0 linux-leds/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-leds linux-leds/ https://lore.kernel.org/linux-leds \
		linux-leds@vger.kernel.org linux-leds@archiver.kernel.org
	public-inbox-index linux-leds

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-leds


AGPL code for this site: git clone https://public-inbox.org/ public-inbox