* [PATCH] phy: micrel: add of configuration for LED mode
@ 2014-02-26 11:48 Ben Dooks
2014-02-26 22:00 ` David Miller
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Ben Dooks @ 2014-02-26 11:48 UTC (permalink / raw)
To: linux-kernel, devicetree, netdev; +Cc: f.fainelli, Ben Dooks
Add support for the led-mode property for the following PHYs
which have a single LED mode configuration value.
KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
to control the LED configuration.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Documentation/devicetree/bindings/net/micrel.txt | 18 +++++++++
drivers/net/phy/micrel.c | 49 ++++++++++++++++++++++--
2 files changed, 63 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/micrel.txt
diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
new file mode 100644
index 0000000..98a3e61
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/micrel.txt
@@ -0,0 +1,18 @@
+Micrel PHY properties.
+
+These properties cover the base properties Micrel PHYs.
+
+Optional properties:
+
+ - micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
+
+ Configure the LED mode with single value. The list of PHYs and
+ the bits that are currently supported:
+
+ KSZ8001: register 0x1e, bits 15..14
+ KSZ8041: register 0x1e, bits 15..14
+ KSZ8021: register 0x1f, bits 5..4
+ KSZ8031: register 0x1f, bits 5..4
+ KSZ8051: register 0x1f, bits 5..4
+
+ See the respective PHY datasheet for the mode values.
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 5a8993b..0c9e434 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
return rc < 0 ? rc : 0;
}
+static int kszphy_setup_led(struct phy_device *phydev,
+ unsigned int reg, unsigned int shift)
+{
+
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+ int rc, temp;
+ u32 val;
+
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
+
+ if (of_property_read_u32(of_node, "micrel,led-mode", &val))
+ return 0;
+
+ temp = phy_read(phydev, reg);
+ if (temp < 0)
+ return temp;
+
+ temp &= 3 << shift;
+ temp |= val << shift;
+ rc = phy_write(phydev, reg, temp);
+
+ return rc < 0 ? rc : 0;
+}
+
static int kszphy_config_init(struct phy_device *phydev)
{
return 0;
}
+static int kszphy_config_init_led8041(struct phy_device *phydev)
+{
+ /* single led control, register 0x1e bits 15..14 */
+ return kszphy_setup_led(phydev, 0x1e, 14);
+}
+
static int ksz8021_config_init(struct phy_device *phydev)
{
- int rc;
const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
+ int rc;
+
+ rc = kszphy_setup_led(phydev, 0x1f, 4);
+ if (rc)
+ dev_err(&phydev->dev, "failed to set led mode\n");
+
phy_write(phydev, MII_KSZPHY_OMSO, val);
rc = ksz_config_flags(phydev);
return rc < 0 ? rc : 0;
@@ -166,6 +203,10 @@ static int ks8051_config_init(struct phy_device *phydev)
{
int rc;
+ rc = kszphy_setup_led(phydev, 0x1f, 4);
+ if (rc)
+ dev_err(&phydev->dev, "failed to set led mode\n");
+
rc = ksz_config_flags(phydev);
return rc < 0 ? rc : 0;
}
@@ -327,7 +368,7 @@ static struct phy_driver ksphy_driver[] = {
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
| SUPPORTED_Asym_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init,
+ .config_init = kszphy_config_init_led8041,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -342,7 +383,7 @@ static struct phy_driver ksphy_driver[] = {
.features = PHY_BASIC_FEATURES |
SUPPORTED_Pause | SUPPORTED_Asym_Pause,
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init,
+ .config_init = kszphy_config_init_led8041,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
@@ -371,7 +412,7 @@ static struct phy_driver ksphy_driver[] = {
.phy_id_mask = 0x00ffffff,
.features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
.flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
- .config_init = kszphy_config_init,
+ .config_init = kszphy_config_init_led8041,
.config_aneg = genphy_config_aneg,
.read_status = genphy_read_status,
.ack_interrupt = kszphy_ack_interrupt,
--
1.8.5.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-26 11:48 [PATCH] phy: micrel: add of configuration for LED mode Ben Dooks
@ 2014-02-26 22:00 ` David Miller
2014-02-26 22:20 ` Florian Fainelli
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2014-02-26 22:00 UTC (permalink / raw)
To: ben.dooks; +Cc: linux-kernel, devicetree, netdev, f.fainelli
From: Ben Dooks <ben.dooks@codethink.co.uk>
Date: Wed, 26 Feb 2014 11:48:00 +0000
> Add support for the led-mode property for the following PHYs
> which have a single LED mode configuration value.
>
> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
> to control the LED configuration.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Applied, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-26 11:48 [PATCH] phy: micrel: add of configuration for LED mode Ben Dooks
2014-02-26 22:00 ` David Miller
@ 2014-02-26 22:20 ` Florian Fainelli
2014-02-27 10:22 ` Ben Dooks
2014-02-27 9:15 ` Mark Rutland
2014-03-18 20:39 ` Sergei Shtylyov
3 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2014-02-26 22:20 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-kernel, devicetree@vger.kernel.org, netdev
Hi,
2014-02-26 3:48 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
[snip]
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 5a8993b..0c9e434 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
> return rc < 0 ? rc : 0;
> }
>
> +static int kszphy_setup_led(struct phy_device *phydev,
> + unsigned int reg, unsigned int shift)
> +{
> +
> + struct device *dev = &phydev->dev;
> + struct device_node *of_node = dev->of_node;
> + int rc, temp;
> + u32 val;
> +
> + if (!of_node && dev->parent->of_node)
> + of_node = dev->parent->of_node;
> +
> + if (of_property_read_u32(of_node, "micrel,led-mode", &val))
> + return 0;
This breaks non-OF configuration because of_read_property_read_u32()
will return -ENOSYS, so you skip the LED register configuration
entirely, is that intended?
> +
> + temp = phy_read(phydev, reg);
> + if (temp < 0)
> + return temp;
> +
> + temp &= 3 << shift;
The compiler cannot verify that we are not overflowing, you might want
to make sure that shift <= 14 (just in case)
> + temp |= val << shift;
> + rc = phy_write(phydev, reg, temp);
> +
> + return rc < 0 ? rc : 0;
You could have;
return phy_write(phydev, reg, temp);
> +}
> +
> static int kszphy_config_init(struct phy_device *phydev)
> {
> return 0;
> }
>
> +static int kszphy_config_init_led8041(struct phy_device *phydev)
> +{
> + /* single led control, register 0x1e bits 15..14 */
> + return kszphy_setup_led(phydev, 0x1e, 14);
> +}
> +
> static int ksz8021_config_init(struct phy_device *phydev)
> {
> - int rc;
> const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
> + int rc;
> +
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> phy_write(phydev, MII_KSZPHY_OMSO, val);
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> @@ -166,6 +203,10 @@ static int ks8051_config_init(struct phy_device *phydev)
> {
> int rc;
>
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
> +
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> }
> @@ -327,7 +368,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause
> | SUPPORTED_Asym_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -342,7 +383,7 @@ static struct phy_driver ksphy_driver[] = {
> .features = PHY_BASIC_FEATURES |
> SUPPORTED_Pause | SUPPORTED_Asym_Pause,
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> @@ -371,7 +412,7 @@ static struct phy_driver ksphy_driver[] = {
> .phy_id_mask = 0x00ffffff,
> .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause),
> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
> - .config_init = kszphy_config_init,
> + .config_init = kszphy_config_init_led8041,
> .config_aneg = genphy_config_aneg,
> .read_status = genphy_read_status,
> .ack_interrupt = kszphy_ack_interrupt,
> --
> 1.8.5.3
>
--
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-26 22:20 ` Florian Fainelli
@ 2014-02-27 10:22 ` Ben Dooks
0 siblings, 0 replies; 12+ messages in thread
From: Ben Dooks @ 2014-02-27 10:22 UTC (permalink / raw)
To: Florian Fainelli; +Cc: linux-kernel, devicetree@vger.kernel.org, netdev
On 26/02/14 22:20, Florian Fainelli wrote:
> Hi,
>
> 2014-02-26 3:48 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>
> [snip]
>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 5a8993b..0c9e434 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
>> return rc < 0 ? rc : 0;
>> }
>>
>> +static int kszphy_setup_led(struct phy_device *phydev,
>> + unsigned int reg, unsigned int shift)
>> +{
>> +
>> + struct device *dev = &phydev->dev;
>> + struct device_node *of_node = dev->of_node;
>> + int rc, temp;
>> + u32 val;
>> +
>> + if (!of_node && dev->parent->of_node)
>> + of_node = dev->parent->of_node;
>> +
>> + if (of_property_read_u32(of_node, "micrel,led-mode", &val))
>> + return 0;
>
> This breaks non-OF configuration because of_read_property_read_u32()
> will return -ENOSYS, so you skip the LED register configuration
> entirely, is that intended?
Yes, it is only here for OF case. I should however check if
of_node is available for the !OF case.
>> +
>> + temp = phy_read(phydev, reg);
>> + if (temp < 0)
>> + return temp;
>> +
>> + temp &= 3 << shift;
>
> The compiler cannot verify that we are not overflowing, you might want
> to make sure that shift <= 14 (just in case)
Ok, should I add a WARN_ON(shift > 14) ?
>> + temp |= val << shift;
>> + rc = phy_write(phydev, reg, temp);
>> +
>> + return rc < 0 ? rc : 0;
>
> You could have;
>
> return phy_write(phydev, reg, temp);
Thanks, will change to doing that.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-26 11:48 [PATCH] phy: micrel: add of configuration for LED mode Ben Dooks
2014-02-26 22:00 ` David Miller
2014-02-26 22:20 ` Florian Fainelli
@ 2014-02-27 9:15 ` Mark Rutland
2014-02-27 10:30 ` Ben Dooks
2014-03-18 20:39 ` Sergei Shtylyov
3 siblings, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2014-02-27 9:15 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-kernel@lists.codethink.co.uk, devicetree@vger.kernel.org,
netdev@vger.kernel.org, f.fainelli@gmail.com
On Wed, Feb 26, 2014 at 11:48:00AM +0000, Ben Dooks wrote:
> Add support for the led-mode property for the following PHYs
> which have a single LED mode configuration value.
>
> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
> to control the LED configuration.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Documentation/devicetree/bindings/net/micrel.txt | 18 +++++++++
> drivers/net/phy/micrel.c | 49 ++++++++++++++++++++++--
> 2 files changed, 63 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/micrel.txt
>
> diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> new file mode 100644
> index 0000000..98a3e61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/micrel.txt
> @@ -0,0 +1,18 @@
> +Micrel PHY properties.
> +
> +These properties cover the base properties Micrel PHYs.
> +
> +Optional properties:
> +
> + - micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
> +
> + Configure the LED mode with single value. The list of PHYs and
> + the bits that are currently supported:
> +
> + KSZ8001: register 0x1e, bits 15..14
> + KSZ8041: register 0x1e, bits 15..14
> + KSZ8021: register 0x1f, bits 5..4
> + KSZ8031: register 0x1f, bits 5..4
> + KSZ8051: register 0x1f, bits 5..4
> +
> + See the respective PHY datasheet for the mode values.
What do these mean, roughly,, and why can the kernel not decide how to
cnofigure these?
In general we prefer to not place raw register values in the DT, and I'd
like to know why we'd have to here.
Cheers,
Mark.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-27 9:15 ` Mark Rutland
@ 2014-02-27 10:30 ` Ben Dooks
[not found] ` <530F13DB.90009-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
0 siblings, 1 reply; 12+ messages in thread
From: Ben Dooks @ 2014-02-27 10:30 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel@lists.codethink.co.uk, devicetree@vger.kernel.org,
netdev@vger.kernel.org, f.fainelli@gmail.com
On 27/02/14 09:15, Mark Rutland wrote:
> On Wed, Feb 26, 2014 at 11:48:00AM +0000, Ben Dooks wrote:
>> Add support for the led-mode property for the following PHYs
>> which have a single LED mode configuration value.
>>
>> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
>> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
>> to control the LED configuration.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> Documentation/devicetree/bindings/net/micrel.txt | 18 +++++++++
>> drivers/net/phy/micrel.c | 49 ++++++++++++++++++++++--
>> 2 files changed, 63 insertions(+), 4 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/net/micrel.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
>> new file mode 100644
>> index 0000000..98a3e61
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/micrel.txt
>> @@ -0,0 +1,18 @@
>> +Micrel PHY properties.
>> +
>> +These properties cover the base properties Micrel PHYs.
>> +
>> +Optional properties:
>> +
>> + - micrel,led-mode : LED mode value to set for PHYs with configurable LEDs.
>> +
>> + Configure the LED mode with single value. The list of PHYs and
>> + the bits that are currently supported:
>> +
>> + KSZ8001: register 0x1e, bits 15..14
>> + KSZ8041: register 0x1e, bits 15..14
>> + KSZ8021: register 0x1f, bits 5..4
>> + KSZ8031: register 0x1f, bits 5..4
>> + KSZ8051: register 0x1f, bits 5..4
>> +
>> + See the respective PHY datasheet for the mode values.
>
> What do these mean, roughly,, and why can the kernel not decide how to
> cnofigure these?
Board specific, in the case of the Lager one of the LEDs is connected
to the ethernet mac block to indicate link, however the default mode
is not for just "Link" so we have to change it.
> In general we prefer to not place raw register values in the DT, and I'd
> like to know why we'd have to here.
I could copy out stuff from the data-sheet, but I was trying to avoid a
copy and paste job.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-02-26 11:48 [PATCH] phy: micrel: add of configuration for LED mode Ben Dooks
` (2 preceding siblings ...)
2014-02-27 9:15 ` Mark Rutland
@ 2014-03-18 20:39 ` Sergei Shtylyov
2014-03-18 23:15 ` Sergei Shtylyov
3 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2014-03-18 20:39 UTC (permalink / raw)
To: Ben Dooks, linux-kernel, devicetree, netdev; +Cc: f.fainelli, linux-sh
Hello.
On 02/26/2014 02:48 PM, Ben Dooks wrote:
> Add support for the led-mode property for the following PHYs
> which have a single LED mode configuration value.
> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
> to control the LED configuration.
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
[...]
Missed this patch, unfortunately and now it has been merged already... doh.
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 5a8993b..0c9e434 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
> return rc < 0 ? rc : 0;
> }
>
> +static int kszphy_setup_led(struct phy_device *phydev,
> + unsigned int reg, unsigned int shift)
> +{
> +
> + struct device *dev = &phydev->dev;
> + struct device_node *of_node = dev->of_node;
> + int rc, temp;
> + u32 val;
> +
> + if (!of_node && dev->parent->of_node)
> + of_node = dev->parent->of_node;
> +
> + if (of_property_read_u32(of_node, "micrel,led-mode", &val))
> + return 0;
> +
> + temp = phy_read(phydev, reg);
> + if (temp < 0)
> + return temp;
> +
> + temp &= 3 << shift;
Hm, I guess you meant ~(3 << shift), else it wouldn't work as expected if
the LED field is currently non-zero. Also, I think you didn't want to mask off
unrelated bits. I'm surprised nobody has noticed this before...
> + temp |= val << shift;
> + rc = phy_write(phydev, reg, temp);
> +
> + return rc < 0 ? rc : 0;
> +}
> +
> static int kszphy_config_init(struct phy_device *phydev)
> {
> return 0;
> }
>
> +static int kszphy_config_init_led8041(struct phy_device *phydev)
Why not ksz8041_config_init() like other names in this file?
Why mention LED in the name at all?
> +{
> + /* single led control, register 0x1e bits 15..14 */
> + return kszphy_setup_led(phydev, 0x1e, 14);
> +}
> +
> static int ksz8021_config_init(struct phy_device *phydev)
> {
> - int rc;
> const u16 val = KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE;
> + int rc;
> +
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
LED.
> +
> phy_write(phydev, MII_KSZPHY_OMSO, val);
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> @@ -166,6 +203,10 @@ static int ks8051_config_init(struct phy_device *phydev)
> {
> int rc;
>
> + rc = kszphy_setup_led(phydev, 0x1f, 4);
> + if (rc)
> + dev_err(&phydev->dev, "failed to set led mode\n");
LED.
> +
> rc = ksz_config_flags(phydev);
> return rc < 0 ? rc : 0;
> }
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] phy: micrel: add of configuration for LED mode
2014-03-18 20:39 ` Sergei Shtylyov
@ 2014-03-18 23:15 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2014-03-18 23:15 UTC (permalink / raw)
To: Ben Dooks, linux-kernel, devicetree, netdev; +Cc: f.fainelli, linux-sh
On 03/18/2014 11:39 PM, Sergei Shtylyov wrote:
>> Add support for the led-mode property for the following PHYs
>> which have a single LED mode configuration value.
>> KSZ8001 and KSZ8041 which both use register 0x1e bits 15,14 and
>> KSZ8021, KSZ8031 and KSZ8051 which use register 0x1f bits 5,4
>> to control the LED configuration.
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> [...]
> Missed this patch, unfortunately and now it has been merged already... doh.
Great work, BTW -- looks like you had to browse a lot of datasheets to do
this patch.
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 5a8993b..0c9e434 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -148,15 +148,52 @@ static int ks8737_config_intr(struct phy_device *phydev)
>> return rc < 0 ? rc : 0;
>> }
>>
>> +static int kszphy_setup_led(struct phy_device *phydev,
>> + unsigned int reg, unsigned int shift)
>> +{
>> +
>> + struct device *dev = &phydev->dev;
>> + struct device_node *of_node = dev->of_node;
>> + int rc, temp;
>> + u32 val;
>> +
>> + if (!of_node && dev->parent->of_node)
>> + of_node = dev->parent->of_node;
>> +
>> + if (of_property_read_u32(of_node, "micrel,led-mode", &val))
>> + return 0;
>> +
>> + temp = phy_read(phydev, reg);
>> + if (temp < 0)
>> + return temp;
>> +
>> + temp &= 3 << shift;
> Hm, I guess you meant ~(3 << shift), else it wouldn't work as expected if
> the LED field is currently non-zero. Also, I think you didn't want to mask off
> unrelated bits. I'm surprised nobody has noticed this before...
OK, I'll go fix this myself.
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-03-18 23:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-26 11:48 [PATCH] phy: micrel: add of configuration for LED mode Ben Dooks
2014-02-26 22:00 ` David Miller
2014-02-26 22:20 ` Florian Fainelli
2014-02-27 10:22 ` Ben Dooks
2014-02-27 9:15 ` Mark Rutland
2014-02-27 10:30 ` Ben Dooks
[not found] ` <530F13DB.90009-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2014-03-18 15:56 ` Laurent Pinchart
2014-03-18 16:11 ` Laurent Pinchart
2014-03-18 16:21 ` Ben Dooks
[not found] ` <53287270.1020908-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
2014-03-18 16:31 ` Laurent Pinchart
2014-03-18 20:39 ` Sergei Shtylyov
2014-03-18 23:15 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).