Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-11 Thread Marek Behún
On Thu, 10 Sep 2020 22:44:54 +0100 Russell King - ARM Linux admin wrote: > On Thu, Sep 10, 2020 at 10:31:12PM +0200, Marek Behun wrote: > > On Thu, 10 Sep 2020 19:34:35 +0100 > > Russell King - ARM Linux admin wrote: > > > > > On Thu, Sep 10, 2020 at 08:31:54PM +0200, Andrew Lunn wrote: >

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-11 Thread Marek Behún
On Fri, 11 Sep 2020 09:12:01 +0200 Matthias Schiffer wrote: > On Thu, 2020-09-10 at 17:00 +0200, Andrew Lunn wrote: > > > I propose that at least these HW modes should be available (and > > > documented) for ethernet PHY controlled LEDs: > > > mode to determine link on: > > > - `link` > >

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-11 Thread Andrew Lunn
> - Do all PHYs support manual setting of the LED level, or are the PHYs > that can only work with HW triggers? There are PHYs with do not have simple on/off. > - Is setting PHY registers always efficiently possible, or should SW > triggers be avoided in certain cases? I'm thinking about setups

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-11 Thread Matthias Schiffer
On Thu, 2020-09-10 at 17:00 +0200, Andrew Lunn wrote: > > I propose that at least these HW modes should be available (and > > documented) for ethernet PHY controlled LEDs: > > mode to determine link on: > > - `link` > > mode for activity (these should blink): > > - `activity` (both rx

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Russell King - ARM Linux admin
On Thu, Sep 10, 2020 at 10:31:12PM +0200, Marek Behun wrote: > On Thu, 10 Sep 2020 19:34:35 +0100 > Russell King - ARM Linux admin wrote: > > > On Thu, Sep 10, 2020 at 08:31:54PM +0200, Andrew Lunn wrote: > > > Generally the driver will default to the hardware reset blink > > > pattern. There

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Pavel Machek
On Wed 2020-09-09 18:25:51, Marek Behún wrote: > This patch adds support for controlling the LEDs connected to several > families of Marvell PHYs via the PHY HW LED trigger API. These families > are: 88E1112, 88E1121R, 88E1240, 88E1340S, 88E1510 and 88E1545. More can > be added. > > This patch

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Marek Behún
On Thu, 10 Sep 2020 15:15:41 +0200 Andrew Lunn wrote: > On Thu, Sep 10, 2020 at 02:23:41PM +0200, Pavel Machek wrote: > > On Wed 2020-09-09 18:25:51, Marek Behún wrote: > > > This patch adds support for controlling the LEDs connected to > > > several families of Marvell PHYs via the PHY HW LED

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Marek Behun
On Thu, 10 Sep 2020 22:23:45 +0200 Pavel Machek wrote: > Hi! > > > Okay, so the netdev trigger offers modes `link`, `rx`, `tx`. > > You can enable/disable either of these (via separate sysfs files). `rx` > > and `tx` blink the LED, `link` turns the LED on if the interface is > > linked. > >

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Marek Behun
On Thu, 10 Sep 2020 19:34:35 +0100 Russell King - ARM Linux admin wrote: > On Thu, Sep 10, 2020 at 08:31:54PM +0200, Andrew Lunn wrote: > > Generally the driver will default to the hardware reset blink > > pattern. There are a few PHY drivers which change this at probe, but > > not many. The

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Pavel Machek
Hi! > > We already have different support for blinking in LED subsystem. Lets use > > that. > > You are assuming we have full software control of the LED, we can turn > it on and off. That is not always the case. But there is sometimes a > mode which the hardware blinks the LED. Please see

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Andrew Lunn
> We already have different support for blinking in LED subsystem. Lets use > that. You are assuming we have full software control of the LED, we can turn it on and off. That is not always the case. But there is sometimes a mode which the hardware blinks the LED. Being able to blink the LED is

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Pavel Machek
Hi! > Okay, so the netdev trigger offers modes `link`, `rx`, `tx`. > You can enable/disable either of these (via separate sysfs files). `rx` > and `tx` blink the LED, `link` turns the LED on if the interface is > linked. I wonder if people really need separate rx and tx, but... this sounds

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Andrew Lunn
> I propose that at least these HW modes should be available (and > documented) for ethernet PHY controlled LEDs: > mode to determine link on: > - `link` > mode for activity (these should blink): > - `activity` (both rx and tx), `rx`, `tx` > mode for link (on) and activity (blink) >

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Russell King - ARM Linux admin
On Thu, Sep 10, 2020 at 08:31:54PM +0200, Andrew Lunn wrote: > Generally the driver will default to the hardware reset blink > pattern. There are a few PHY drivers which change this at probe, but > not many. The silicon defaults are pretty good. The "right" blink pattern can be a matter of how

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Andrew Lunn
On Thu, Sep 10, 2020 at 08:24:34PM +0200, Pavel Machek wrote: > On Thu 2020-09-10 15:15:41, Andrew Lunn wrote: > > On Thu, Sep 10, 2020 at 02:23:41PM +0200, Pavel Machek wrote: > > > On Wed 2020-09-09 18:25:51, Marek Behún wrote: > > > > This patch adds support for controlling the LEDs connected

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Pavel Machek
On Thu 2020-09-10 15:15:41, Andrew Lunn wrote: > On Thu, Sep 10, 2020 at 02:23:41PM +0200, Pavel Machek wrote: > > On Wed 2020-09-09 18:25:51, Marek Behún wrote: > > > This patch adds support for controlling the LEDs connected to several > > > families of Marvell PHYs via the PHY HW LED trigger

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Andrew Lunn
> Moreover I propose (and am willing to do) this: > Rewrite phy_led_trigger so that it registers one trigger, `phydev`. > The identifier of the PHY which should be source of the trigger can be > set via a separate sysfs file, `device_name`, like in netdev trigger. > The linked speed on

Re: [PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-10 Thread Andrew Lunn
On Thu, Sep 10, 2020 at 02:23:41PM +0200, Pavel Machek wrote: > On Wed 2020-09-09 18:25:51, Marek Behún wrote: > > This patch adds support for controlling the LEDs connected to several > > families of Marvell PHYs via the PHY HW LED trigger API. These families > > are: 88E1112, 88E1121R, 88E1240,

[PATCH net-next + leds v2 6/7] net: phy: marvell: add support for LEDs controlled by Marvell PHYs

2020-09-09 Thread Marek Behún
This patch adds support for controlling the LEDs connected to several families of Marvell PHYs via the PHY HW LED trigger API. These families are: 88E1112, 88E1121R, 88E1240, 88E1340S, 88E1510 and 88E1545. More can be added. This patch does not yet add support for compound LED modes. This could