[PATCH v2 1/2] net, can, ifi: fix "write buffer full" error
the driver reads in the ISR first the IRQpending register,
and clears after that in a write *all* bits in it.
It could happen that the isr register raise bits between
this 2 register accesses, which leads in lost bits ...
In case it clears "TX message sent successfully", the driver
never sends any Tx data, and buffers to userspace run over.
Fixed this:
clear only the bits in the IRQpending register, the
driver had read.
Signed-off-by: Heiko Schocher
Reviewed-by: Marek Vasut
---
Changes in v2:
- add Reviewed-by from Marek
drivers/net/can/ifi_canfd/ifi_canfd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2772d05ff11c..05feb8177936 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -607,7 +607,7 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
return IRQ_NONE;
/* Clear all pending interrupts but ErrWarn */
- writel(clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
+ writel(isr & clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
/* RX IRQ or bus warning, start NAPI */
if (isr & rx_irq_mask) {
--
2.14.3
[PATCH v2 2/2] net, can, ifi: loopback Tx message in IFI block
Current ifi driver reads first Rx messages, than loopback
the Tx message, if the IFI_CANFD_INTERRUPT_TXFIFO_REMOVE
bit is set. This can lead into the case, that Rx messages
overhelm Tx messages!
Fixed this in the following way:
Set in the IFI_CANFD_TXFIFO_DLC register the FN value to
1, so the IFI block loopsback itself the Tx message when
sended correctly on the canfd bus. Only the IFI block can
insert the Tx message in the correct place.
The linux driver now needs only to free the skb, when
the Tx message was sended correctly.
Signed-off-by: Heiko Schocher
Reviewed-by: Marek Vasut
---
Changes in v2:
- add Reviewed-by from Marek, fixed comment into one liner
drivers/net/can/ifi_canfd/ifi_canfd.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 05feb8177936..ee74ee8f9b38 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -211,6 +211,7 @@ struct ifi_canfd_priv {
struct napi_struct napi;
struct net_device *ndev;
void __iomem*base;
+ unsigned inttx_len;
};
static void ifi_canfd_irq_enable(struct net_device *ndev, bool enable)
@@ -617,8 +618,10 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
/* TX IRQ */
if (isr & IFI_CANFD_INTERRUPT_TXFIFO_REMOVE) {
- stats->tx_bytes += can_get_echo_skb(ndev, 0);
+ can_free_echo_skb(ndev, 0);
+ stats->tx_bytes += priv->tx_len;
stats->tx_packets++;
+ priv->tx_len = 0;
can_led_event(ndev, CAN_LED_EVENT_TX);
}
@@ -889,6 +892,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
}
txdlc = can_len2dlc(cf->len);
+ priv->tx_len = txdlc;
if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
if (cf->flags & CANFD_BRS)
@@ -898,6 +902,9 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
if (cf->can_id & CAN_RTR_FLAG)
txdlc |= IFI_CANFD_TXFIFO_DLC_RTR;
+ /* set FNR to 1, so we get our Tx Message looped back into RxFIFO */
+ txdlc += (1 << IFI_CANFD_TXFIFO_DLC_FNR_OFFSET);
+
/* message ram configuration */
writel(txid, priv->base + IFI_CANFD_TXFIFO_ID);
writel(txdlc, priv->base + IFI_CANFD_TXFIFO_DLC);
--
2.14.3
[PATCH 2/2] net, can, ifi: loopback Tx message in IFI block
Current ifi driver reads first Rx messages, than loopback
the Tx message, if the IFI_CANFD_INTERRUPT_TXFIFO_REMOVE
bit is set. This can lead into the case, that Rx messages
overhelm Tx messages!
Fixed this in the following way:
Set in the IFI_CANFD_TXFIFO_DLC register the FN value to
1, so the IFI block loopsback itself the Tx message when
sended correctly on the canfd bus. Only the IFI block can
insert the Tx message in the correct place.
The linux driver now needs only to free the skb, when
the Tx message was sended correctly.
Signed-off-by: Heiko Schocher
---
drivers/net/can/ifi_canfd/ifi_canfd.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 05feb8177936..0d36cb8659ae 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -211,6 +211,7 @@ struct ifi_canfd_priv {
struct napi_struct napi;
struct net_device *ndev;
void __iomem*base;
+ unsigned inttx_len;
};
static void ifi_canfd_irq_enable(struct net_device *ndev, bool enable)
@@ -617,8 +618,10 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
/* TX IRQ */
if (isr & IFI_CANFD_INTERRUPT_TXFIFO_REMOVE) {
- stats->tx_bytes += can_get_echo_skb(ndev, 0);
+ can_free_echo_skb(ndev, 0);
+ stats->tx_bytes += priv->tx_len;
stats->tx_packets++;
+ priv->tx_len = 0;
can_led_event(ndev, CAN_LED_EVENT_TX);
}
@@ -889,6 +892,7 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff *skb,
}
txdlc = can_len2dlc(cf->len);
+ priv->tx_len = txdlc;
if ((priv->can.ctrlmode & CAN_CTRLMODE_FD) && can_is_canfd_skb(skb)) {
txdlc |= IFI_CANFD_TXFIFO_DLC_EDL;
if (cf->flags & CANFD_BRS)
@@ -898,6 +902,12 @@ static netdev_tx_t ifi_canfd_start_xmit(struct sk_buff
*skb,
if (cf->can_id & CAN_RTR_FLAG)
txdlc |= IFI_CANFD_TXFIFO_DLC_RTR;
+ /*
+* set FNR to 1, so we get our Tx Message looped back
+* into RxFIFO
+*/
+ txdlc += (1 << IFI_CANFD_TXFIFO_DLC_FNR_OFFSET);
+
/* message ram configuration */
writel(txid, priv->base + IFI_CANFD_TXFIFO_ID);
writel(txdlc, priv->base + IFI_CANFD_TXFIFO_DLC);
--
2.14.3
[PATCH 1/2] net, can, ifi: fix "write buffer full" error
the driver reads in the ISR first the IRQpending register,
and clears after that in a write *all* bits in it.
It could happen that the isr register raise bits between
this 2 register accesses, which leads in lost bits ...
In case it clears "TX message sent successfully", the driver
never sends any Tx data, and buffers to userspace run over.
Fixed this:
clear only the bits in the IRQpending register, the
driver had read.
Signed-off-by: Heiko Schocher
---
drivers/net/can/ifi_canfd/ifi_canfd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c
b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2772d05ff11c..05feb8177936 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -607,7 +607,7 @@ static irqreturn_t ifi_canfd_isr(int irq, void *dev_id)
return IRQ_NONE;
/* Clear all pending interrupts but ErrWarn */
- writel(clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
+ writel(isr & clr_irq_mask, priv->base + IFI_CANFD_INTERRUPT);
/* RX IRQ or bus warning, start NAPI */
if (isr & rx_irq_mask) {
--
2.14.3
Re: [PATCH] Make EN2 pin optional in the TRF7970A driver
Hello all, Am 21.02.2017 um 17:43 schrieb Rob Herring: On Sun, Feb 19, 2017 at 11:19 PM, Heiko Schocher wrote: Hello all, Am 13.02.2017 um 22:31 schrieb Rob Herring: On Mon, Feb 13, 2017 at 12:38 AM, Heiko Schocher wrote: Hello Rob, Am 10.02.2017 um 16:51 schrieb Rob Herring: On Tue, Feb 07, 2017 at 06:22:04AM +0100, Heiko Schocher wrote: From: Guan Ben Make the EN2 pin optional. This is useful for boards, which have this pin fix wired, for example to ground. Signed-off-by: Guan Ben Signed-off-by: Mark Jonas Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++-- drivers/nfc/trf7970a.c | 26 -- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt index 32b35a0..5889a3d 100644 --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt @@ -5,8 +5,8 @@ Required properties: - spi-max-frequency: Maximum SPI frequency (<= 200). - interrupt-parent: phandle of parent interrupt handler. - interrupts: A single interrupt specifier. -- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the - TRF7970A. +- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the + TRF7970A. EN2 is optional. Could EN ever be optional/fixed? If so, perhaps deprecate this property and do 2 properties, one for each pin. The hardware I have has the EN2 pin fix connected to ground. Looking into http://www.ti.com/lit/ds/slos743k/slos743k.pdf page 19 table 6-3 and 6-4 the EN2 pin is a don;t core if EN = 1. If EN = 0 EN2 pin selects between Power Down and Sleep Mode ... I see no reason why this is not possible/allowed ... Hmm.. I do not like the idea of deprecating the "ti,enable-gpios" property into 2 seperate properties ... but if this would be a reason for not accepting this patch, I can do this ... How should I name the 2 new properties? I guess if this ever happens, then we just add "ti,enable2-gpios" and ti,enable-gpios continues to point to EN. We don't need to deprecate anything (or maybe just deprecate having both GPIOs on single property). In that case, Acked-by: Rob Herring gentle ping. Are there any more comments to this patch? Is it acceptable as it is? I acked it, so yes, it is fine. Gentle ping. Any more issues or can this patch go into mainline? bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [PATCH] Make EN2 pin optional in the TRF7970A driver
Hello all, Am 13.02.2017 um 22:31 schrieb Rob Herring: On Mon, Feb 13, 2017 at 12:38 AM, Heiko Schocher wrote: Hello Rob, Am 10.02.2017 um 16:51 schrieb Rob Herring: On Tue, Feb 07, 2017 at 06:22:04AM +0100, Heiko Schocher wrote: From: Guan Ben Make the EN2 pin optional. This is useful for boards, which have this pin fix wired, for example to ground. Signed-off-by: Guan Ben Signed-off-by: Mark Jonas Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++-- drivers/nfc/trf7970a.c | 26 -- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt index 32b35a0..5889a3d 100644 --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt @@ -5,8 +5,8 @@ Required properties: - spi-max-frequency: Maximum SPI frequency (<= 200). - interrupt-parent: phandle of parent interrupt handler. - interrupts: A single interrupt specifier. -- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the - TRF7970A. +- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the + TRF7970A. EN2 is optional. Could EN ever be optional/fixed? If so, perhaps deprecate this property and do 2 properties, one for each pin. The hardware I have has the EN2 pin fix connected to ground. Looking into http://www.ti.com/lit/ds/slos743k/slos743k.pdf page 19 table 6-3 and 6-4 the EN2 pin is a don;t core if EN = 1. If EN = 0 EN2 pin selects between Power Down and Sleep Mode ... I see no reason why this is not possible/allowed ... Hmm.. I do not like the idea of deprecating the "ti,enable-gpios" property into 2 seperate properties ... but if this would be a reason for not accepting this patch, I can do this ... How should I name the 2 new properties? I guess if this ever happens, then we just add "ti,enable2-gpios" and ti,enable-gpios continues to point to EN. We don't need to deprecate anything (or maybe just deprecate having both GPIOs on single property). In that case, Acked-by: Rob Herring gentle ping. Are there any more comments to this patch? Is it acceptable as it is? Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [PATCH] Make EN2 pin optional in the TRF7970A driver
Hello Rob, Am 10.02.2017 um 16:51 schrieb Rob Herring: On Tue, Feb 07, 2017 at 06:22:04AM +0100, Heiko Schocher wrote: From: Guan Ben Make the EN2 pin optional. This is useful for boards, which have this pin fix wired, for example to ground. Signed-off-by: Guan Ben Signed-off-by: Mark Jonas Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++-- drivers/nfc/trf7970a.c | 26 -- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt index 32b35a0..5889a3d 100644 --- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt +++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt @@ -5,8 +5,8 @@ Required properties: - spi-max-frequency: Maximum SPI frequency (<= 200). - interrupt-parent: phandle of parent interrupt handler. - interrupts: A single interrupt specifier. -- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the - TRF7970A. +- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the + TRF7970A. EN2 is optional. Could EN ever be optional/fixed? If so, perhaps deprecate this property and do 2 properties, one for each pin. The hardware I have has the EN2 pin fix connected to ground. Looking into http://www.ti.com/lit/ds/slos743k/slos743k.pdf page 19 table 6-3 and 6-4 the EN2 pin is a don;t core if EN = 1. If EN = 0 EN2 pin selects between Power Down and Sleep Mode ... I see no reason why this is not possible/allowed ... Hmm.. I do not like the idea of deprecating the "ti,enable-gpios" property into 2 seperate properties ... but if this would be a reason for not accepting this patch, I can do this ... How should I name the 2 new properties? "ti,pin-enable" and "ti,pin-enable2" ? bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [net, v3, 1/3] net: phy: Fix PHY module checks and NULL deref in phy_attach_direct()
Hello Florian,
Am 09.02.2017 um 08:13 schrieb Florian Fainelli:
On 02/08/2017 10:58 PM, Heiko Schocher wrote:
Hello Florian,
Am 09.02.2017 um 01:13 schrieb Florian Fainelli:
The Generic PHY drivers gets assigned after we checked that the current
PHY driver is NULL, so we need to check a few things before we can
safely dereference d->driver. This would be causing a NULL deference to
occur when a system binds to the Generic PHY driver. Update
phy_attach_direct() to do the following:
- grab the driver module reference after we have assigned the Generic
PHY drivers accordingly
- update the error path to clean up the module reference in case the
Generic PHY probe function fails
Fixes: cafe8df8b9bc ("net: phy: Fix lack of reference count on PHY
driver")
Signed-off-by: Florian Fainelli
---
drivers/net/phy/phy_device.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
just stumbled over this bug on an am335x based board, with an
KSZ8081 attached, so there a "fixed-link" is used like:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-baltos-ir3220.dts#n105
With your patch it crashes also ...
The final version of the patch is here:
http://patchwork.ozlabs.org/patch/725923/
Huh, sorry ...
Do you mind giving it a try?
With this patch, ethernet works again fine on this board, thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Re: [net, v3, 1/3] net: phy: Fix PHY module checks and NULL deref in phy_attach_direct()
Hello Florian,
Am 09.02.2017 um 01:13 schrieb Florian Fainelli:
The Generic PHY drivers gets assigned after we checked that the current
PHY driver is NULL, so we need to check a few things before we can
safely dereference d->driver. This would be causing a NULL deference to
occur when a system binds to the Generic PHY driver. Update
phy_attach_direct() to do the following:
- grab the driver module reference after we have assigned the Generic
PHY drivers accordingly
- update the error path to clean up the module reference in case the
Generic PHY probe function fails
Fixes: cafe8df8b9bc ("net: phy: Fix lack of reference count on PHY driver")
Signed-off-by: Florian Fainelli
---
drivers/net/phy/phy_device.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
just stumbled over this bug on an am335x based board, with an
KSZ8081 attached, so there a "fixed-link" is used like:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-baltos-ir3220.dts#n105
With your patch it crashes also ...
If I remove this part:
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index d63d190..9dd08a4 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -921,11 +921,6 @@ int phy_attach_direct(struct net_device *dev, struct
phy_device *phydev,
return -EIO;
}
- if (!try_module_get(d->driver->owner)) {
- dev_err(&dev->dev, "failed to get the device driver module\n");
- return -EIO;
- }
-
get_device(d);
/* Assume that if there is no driver, that it doesn't
it boots again .. I think, you forgot? simply this remove ?
bye,
Heiko
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 0d8f4d3847f6..d63d190a95ef 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -908,6 +908,7 @@ int phy_attach_direct(struct net_device *dev, struct
phy_device *phydev,
struct module *ndev_owner = dev->dev.parent->driver->owner;
struct mii_bus *bus = phydev->mdio.bus;
struct device *d = &phydev->mdio.dev;
+ bool using_genphy = false;
int err;
/* For Ethernet device drivers that register their own MDIO bus, we
@@ -938,12 +939,22 @@ int phy_attach_direct(struct net_device *dev, struct
phy_device *phydev,
d->driver =
&genphy_driver[GENPHY_DRV_1G].mdiodrv.driver;
+ using_genphy = true;
+ }
+
+ if (!try_module_get(d->driver->owner)) {
+ dev_err(&dev->dev, "failed to get the device driver module\n");
+ err = -EIO;
+ goto error_put_device;
+ }
+
+ if (using_genphy) {
err = d->driver->probe(d);
if (err >= 0)
err = device_bind_driver(d);
if (err)
- goto error;
+ goto error_module_put;
}
if (phydev->attached_dev) {
@@ -981,6 +992,9 @@ int phy_attach_direct(struct net_device *dev, struct
phy_device *phydev,
error:
phy_detach(phydev);
+error_module_put:
+ module_put(d->driver->owner);
+error_put_device:
put_device(d);
module_put(d->driver->owner);
if (ndev_owner != bus->owner)
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
[PATCH] Make EN2 pin optional in the TRF7970A driver
From: Guan Ben
Make the EN2 pin optional. This is useful for boards,
which have this pin fix wired, for example to ground.
Signed-off-by: Guan Ben
Signed-off-by: Mark Jonas
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/nfc/trf7970a.txt | 4 ++--
drivers/nfc/trf7970a.c | 26 --
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
index 32b35a0..5889a3d 100644
--- a/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
+++ b/Documentation/devicetree/bindings/net/nfc/trf7970a.txt
@@ -5,8 +5,8 @@ Required properties:
- spi-max-frequency: Maximum SPI frequency (<= 200).
- interrupt-parent: phandle of parent interrupt handler.
- interrupts: A single interrupt specifier.
-- ti,enable-gpios: Two GPIO entries used for 'EN' and 'EN2' pins on the
- TRF7970A.
+- ti,enable-gpios: One or two GPIO entries used for 'EN' and 'EN2' pins on the
+ TRF7970A. EN2 is optional.
- vin-supply: Regulator for supply voltage to VIN pin
Optional SoC Specific Properties:
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 26c9dbb..75079fb 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1885,8 +1885,10 @@ static int trf7970a_power_up(struct trf7970a *trf)
usleep_range(5000, 6000);
if (!(trf->quirks & TRF7970A_QUIRK_EN2_MUST_STAY_LOW)) {
- gpio_set_value(trf->en2_gpio, 1);
- usleep_range(1000, 2000);
+ if (gpio_is_valid(trf->en2_gpio)) {
+ gpio_set_value(trf->en2_gpio, 1);
+ usleep_range(1000, 2000);
+ }
}
gpio_set_value(trf->en_gpio, 1);
@@ -1914,7 +1916,8 @@ static int trf7970a_power_down(struct trf7970a *trf)
}
gpio_set_value(trf->en_gpio, 0);
- gpio_set_value(trf->en2_gpio, 0);
+ if (gpio_is_valid(trf->en2_gpio))
+ gpio_set_value(trf->en2_gpio, 0);
ret = regulator_disable(trf->regulator);
if (ret)
@@ -2032,15 +2035,14 @@ static int trf7970a_probe(struct spi_device *spi)
trf->en2_gpio = of_get_named_gpio(np, "ti,enable-gpios", 1);
if (!gpio_is_valid(trf->en2_gpio)) {
- dev_err(trf->dev, "No EN2 GPIO property\n");
- return trf->en2_gpio;
- }
-
- ret = devm_gpio_request_one(trf->dev, trf->en2_gpio,
- GPIOF_DIR_OUT | GPIOF_INIT_LOW, "trf7970a EN2");
- if (ret) {
- dev_err(trf->dev, "Can't request EN2 GPIO: %d\n", ret);
- return ret;
+ dev_info(trf->dev, "No EN2 GPIO property\n");
+ } else {
+ ret = devm_gpio_request_one(trf->dev, trf->en2_gpio,
+ GPIOF_DIR_OUT | GPIOF_INIT_LOW, "trf7970a EN2");
+ if (ret) {
+ dev_err(trf->dev, "Can't request EN2 GPIO: %d\n", ret);
+ return ret;
+ }
}
if (of_property_read_bool(np, "en2-rf-quirk"))
--
2.7.4
Re: [PATCH] net, can, ti_hecc: add DT support for the ti,hecc controller
Hello Marc,
Am 19.10.2015 um 08:58 schrieb Marc Kleine-Budde:
On 10/19/2015 08:39 AM, Heiko Schocher wrote:
add DT support for the ti hecc controller, used on
am3517 SoCs.
A similar patch was posted a few days ago, see
http://comments.gmane.org/gmane.linux.can/8616 and my comments.
Uh, sorry! Seems I missed them ...
Please coordinate with Anton Glukhov (Cc'ed) and/or pick up his patches
as they are in better shape.
Yes, I try the patchset from Anton ... thanks for pointing to them.
@Anton: Do you have a newer version, which contains the comments
from Marc?
bye,
Heiko
Marc
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/can/ti_hecc-can.txt| 20 ++
arch/arm/boot/dts/am3517.dtsi | 13 +++
drivers/net/can/ti_hecc.c | 45 +-
3 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
b/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
new file mode 100644
index 000..09fab59
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
@@ -0,0 +1,20 @@
+* TI HECC CAN *
+
+Required properties:
+ - compatible: Should be "ti,hecc"
We usually put the name of the first SoC this IP core appears in to the
compatible.
Ok, so "ti,am335xx-hecc" would be OK?
@Anton: you used "am35x" ... it should be "am35xx"
+ - reg: Should contain CAN controller registers location and length
+ - interrupts: Should contain IRQ line for the CAN controller
I'm missing the description of the ti,* properties. I think they are
required, too. Although the code doesn't enforce it.
Ok.
+
+Example:
+
+ can0: hecc@5c05 {
+ compatible = "ti,hecc";
+ reg = <0x5c05 0x4000>;
+ interrupts = <24>;
+ ti,hecc_scc_offset = <0>;
+ ti,hecc_scc_ram_offset = <0x3000>;
+ ti,hecc_ram_offset = <0x3000>;
+ ti,hecc_mbx_offset = <0x2000>;
+ ti,hecc_int_line = <0>;
+ ti,hecc_version = <1>;
Versioning in the OF world is done via the compatible. Are the offsets a
per SoC parameter? I'm not sure if it's better to put
the offsets into the driver.
I am unsure here too..
+ };
diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
index 5e3f5e8..47bc429 100644
--- a/arch/arm/boot/dts/am3517.dtsi
+++ b/arch/arm/boot/dts/am3517.dtsi
@@ -25,6 +25,19 @@
interrupt-names = "mc";
};
+ can0: hecc@5c05 {
+ compatible = "ti,hecc";
+ reg = <0x5c05 0x4000>;
+ interrupts = <24>;
+ ti,hecc_scc_offset = <0>;
+ ti,hecc_scc_ram_offset = <0x3000>;
+ ti,hecc_ram_offset = <0x3000>;
+ ti,hecc_mbx_offset = <0x2000>;
+ ti,hecc_int_line = <0>;
+ ti,hecc_version = <1>;
+ status = "disabled";
+ };
+
davinci_emac: ethernet@0x5c00 {
compatible = "ti,am3517-emac";
ti,hwmods = "davinci_emac";
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index c08e8ea..f1705d5 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -875,16 +875,56 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};
+#if defined(CONFIG_OF)
+static const struct of_device_id ti_hecc_can_dt_ids[] = {
+ {
+ .compatible = "ti,hecc",
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, ti_hecc_can_dt_ids);
+#endif
Please remove the ifdef, use __maybe_unused instead.
+
+static const struct ti_hecc_platform_data
+*ti_hecc_can_get_driver_data(struct platform_device *pdev)
+{
+ if (pdev->dev.of_node) {
+ struct ti_hecc_platform_data *data;
+ struct device_node *np = pdev->dev.of_node;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return NULL;
+
+ of_property_read_u32(np, "ti,hecc_scc_offset",
+&data->scc_hecc_offset);
+ of_property_read_u32(np, "ti,hecc_scc_ram_offset",
+&data->scc_ram_offset);
+ of_property_read_u32(np, "ti,hecc_ram_offset",
+
Re: [PATCH] net, can, ti_hecc: fix a run time warn_on.
Hello Marc,
Am 19.10.2015 um 08:34 schrieb Marc Kleine-Budde:
On 10/19/2015 08:22 AM, Heiko Schocher wrote:
This patch fixes a warning in clk_enable by calling
clk_prepare_enable instead.
What about the corresponding clk_disable_unprepare()?
Yes, that should be fixed too, do this in a v2, thanks!
bye,
Heiko
Marc
Signed-off-by: Heiko Schocher
---
drivers/net/can/ti_hecc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index cf345cb..c08e8ea 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -951,7 +951,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll,
HECC_DEF_NAPI_WEIGHT);
- clk_enable(priv->clk);
+ clk_prepare_enable(priv->clk);
err = register_candev(ndev);
if (err) {
dev_err(&pdev->dev, "register_candev() failed\n");
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net, can, ti_hecc: add DT support for the ti,hecc controller
add DT support for the ti hecc controller, used on
am3517 SoCs.
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/can/ti_hecc-can.txt| 20 ++
arch/arm/boot/dts/am3517.dtsi | 13 +++
drivers/net/can/ti_hecc.c | 45 +-
3 files changed, 76 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
diff --git a/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
b/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
new file mode 100644
index 000..09fab59
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/ti_hecc-can.txt
@@ -0,0 +1,20 @@
+* TI HECC CAN *
+
+Required properties:
+ - compatible: Should be "ti,hecc"
+ - reg: Should contain CAN controller registers location and length
+ - interrupts: Should contain IRQ line for the CAN controller
+
+Example:
+
+ can0: hecc@5c05 {
+ compatible = "ti,hecc";
+ reg = <0x5c05 0x4000>;
+ interrupts = <24>;
+ ti,hecc_scc_offset = <0>;
+ ti,hecc_scc_ram_offset = <0x3000>;
+ ti,hecc_ram_offset = <0x3000>;
+ ti,hecc_mbx_offset = <0x2000>;
+ ti,hecc_int_line = <0>;
+ ti,hecc_version = <1>;
+ };
diff --git a/arch/arm/boot/dts/am3517.dtsi b/arch/arm/boot/dts/am3517.dtsi
index 5e3f5e8..47bc429 100644
--- a/arch/arm/boot/dts/am3517.dtsi
+++ b/arch/arm/boot/dts/am3517.dtsi
@@ -25,6 +25,19 @@
interrupt-names = "mc";
};
+ can0: hecc@5c05 {
+ compatible = "ti,hecc";
+ reg = <0x5c05 0x4000>;
+ interrupts = <24>;
+ ti,hecc_scc_offset = <0>;
+ ti,hecc_scc_ram_offset = <0x3000>;
+ ti,hecc_ram_offset = <0x3000>;
+ ti,hecc_mbx_offset = <0x2000>;
+ ti,hecc_int_line = <0>;
+ ti,hecc_version = <1>;
+ status = "disabled";
+ };
+
davinci_emac: ethernet@0x5c00 {
compatible = "ti,am3517-emac";
ti,hwmods = "davinci_emac";
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index c08e8ea..f1705d5 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -875,16 +875,56 @@ static const struct net_device_ops ti_hecc_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};
+#if defined(CONFIG_OF)
+static const struct of_device_id ti_hecc_can_dt_ids[] = {
+ {
+ .compatible = "ti,hecc",
+ }, {
+ /* sentinel */
+ }
+};
+MODULE_DEVICE_TABLE(of, ti_hecc_can_dt_ids);
+#endif
+
+static const struct ti_hecc_platform_data
+*ti_hecc_can_get_driver_data(struct platform_device *pdev)
+{
+ if (pdev->dev.of_node) {
+ struct ti_hecc_platform_data *data;
+ struct device_node *np = pdev->dev.of_node;
+
+ data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return NULL;
+
+ of_property_read_u32(np, "ti,hecc_scc_offset",
+&data->scc_hecc_offset);
+ of_property_read_u32(np, "ti,hecc_scc_ram_offset",
+&data->scc_ram_offset);
+ of_property_read_u32(np, "ti,hecc_ram_offset",
+&data->hecc_ram_offset);
+ of_property_read_u32(np, "ti,hecc_mbx_offset",
+&data->mbx_offset);
+ of_property_read_u32(np, "ti,hecc_int_line",
+&data->int_line);
+ of_property_read_u32(np, "ti,hecc_version",
+&data->version);
+ return data;
+ }
+ return (const struct ti_hecc_platform_data *)
+ dev_get_platdata(&pdev->dev);
+}
+
static int ti_hecc_probe(struct platform_device *pdev)
{
struct net_device *ndev = (struct net_device *)0;
struct ti_hecc_priv *priv;
- struct ti_hecc_platform_data *pdata;
+ const struct ti_hecc_platform_data *pdata;
struct resource *mem, *irq;
void __iomem *addr;
int err = -ENODEV;
- pdata = dev_get_platdata(&pdev->dev);
+ pdata = ti_hecc_can_get_driver_data(pdev);
if (!pdata) {
dev_err(&pdev->dev, "No platform data
[PATCH] net, can, ti_hecc: fix a run time warn_on.
This patch fixes a warning in clk_enable by calling
clk_prepare_enable instead.
Signed-off-by: Heiko Schocher
---
drivers/net/can/ti_hecc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index cf345cb..c08e8ea 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -951,7 +951,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll,
HECC_DEF_NAPI_WEIGHT);
- clk_enable(priv->clk);
+ clk_prepare_enable(priv->clk);
err = register_candev(ndev);
if (err) {
dev_err(&pdev->dev, "register_candev() failed\n");
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] drivers: net: cpsw: add phy-handle parsing
add the ability to parse "phy-handle". This
is needed for phys, which have a DT node, and
need to parse DT properties.
Signed-off-by: Heiko Schocher
---
Changes in v2: None
Documentation/devicetree/bindings/net/cpsw.txt | 1 +
drivers/net/ethernet/ti/cpsw.c | 15 +++
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt
b/Documentation/devicetree/bindings/net/cpsw.txt
index a9df21a..a2cae4e 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -39,6 +39,7 @@ Required properties:
Optional properties:
- dual_emac_res_vlan : Specifies VID to be used to segregate the ports
- mac-address : See ethernet.txt file in the same directory
+- phy-handle : See ethernet.txt file in the same directory
Note: "ti,hwmods" field is used to fetch the base address and irq
resources from TI, omap hwmod data base during device registration.
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 8fc90f1..874fb29 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -365,6 +366,7 @@ struct cpsw_priv {
spinlock_t lock;
struct platform_device *pdev;
struct net_device *ndev;
+ struct device_node *phy_node;
struct napi_struct napi_rx;
struct napi_struct napi_tx;
struct device *dev;
@@ -1145,7 +1147,11 @@ static void cpsw_slave_open(struct cpsw_slave *slave,
struct cpsw_priv *priv)
cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
- slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
+ if (priv->phy_node)
+ slave->phy = of_phy_connect(priv->ndev, priv->phy_node,
+&cpsw_adjust_link, 0, slave->data->phy_if);
+ else
+ slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
&cpsw_adjust_link, slave->data->phy_if);
if (IS_ERR(slave->phy)) {
dev_err(priv->dev, "phy %s not found on slave %d\n",
@@ -1934,11 +1940,12 @@ static void cpsw_slave_init(struct cpsw_slave *slave,
struct cpsw_priv *priv,
slave->port_vlan = data->dual_emac_res_vlan;
}
-static int cpsw_probe_dt(struct cpsw_platform_data *data,
+static int cpsw_probe_dt(struct cpsw_priv *priv,
struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
struct device_node *slave_node;
+ struct cpsw_platform_data *data = &priv->data;
int i = 0, ret;
u32 prop;
@@ -2029,6 +2036,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
if (strcmp(slave_node->name, "slave"))
continue;
+ priv->phy_node = of_parse_phandle(slave_node, "phy-handle", 0);
parp = of_get_property(slave_node, "phy_id", &lenp);
if ((parp == NULL) || (lenp != (sizeof(void *) * 2))) {
dev_err(&pdev->dev, "Missing slave[%d] phy_id
property\n", i);
@@ -2044,7 +2052,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
PHY_ID_FMT, mdio->name, phyid);
-
slave_data->phy_if = of_get_phy_mode(slave_node);
if (slave_data->phy_if < 0) {
dev_err(&pdev->dev, "Missing or malformed slave[%d]
phy-mode property\n",
@@ -2240,7 +2247,7 @@ static int cpsw_probe(struct platform_device *pdev)
/* Select default pin state */
pinctrl_pm_select_default_state(&pdev->dev);
- if (cpsw_probe_dt(&priv->data, pdev)) {
+ if (cpsw_probe_dt(priv, pdev)) {
dev_err(&pdev->dev, "cpsw: platform data missing\n");
ret = -ENODEV;
goto clean_runtime_disable_ret;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] net: phy: smsc: disable energy detect mode
On some boards the energy enable detect mode leads in
trouble with some switches, so make the enabling of
this mode configurable through DT.
Signed-off-by: Heiko Schocher
---
Changes in v2:
- add comments from Florian Fainelli
- I did not change disable property name into enable
because I fear to break existing behaviour
- add smsc vendor prefix
- remove CONFIG_OF and use __maybe_unused
- introduce "phy-handle" ability into ti,cpsw
driver, so I can remove bogus:
if (!of_node && dev->parent->of_node)
of_node = dev->parent->of_node;
construct. Therefore new patch for the ti,cpsw
driver is necessary.
.../devicetree/bindings/net/smsc-lan87xx.txt | 24 ++
drivers/net/phy/smsc.c | 19 -
2 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt
diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
new file mode 100644
index 000..974edd5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -0,0 +1,24 @@
+SMSC LAN87xx Ethernet PHY
+
+Some boards require special tuning values. Configure them
+through an Ethernet OF device node.
+
+Optional properties:
+
+- smsc,disable-energy-detect:
+ If set, do not enable energy detect mode for the SMSC phy.
+ default: enable energy detect mode
+
+Examples:
+smsc phy with disabled energy detect mode on an am335x based board.
+&davinci_mdio {
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&davinci_mdio_default>;
+ pinctrl-1 = <&davinci_mdio_sleep>;
+ status = "okay";
+
+ ethernetphy0: ethernet-phy@0 {
+ reg = <0>;
+ smsc,disable-energy-detect;
+ };
+};
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 70b0895..dc2da87 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,16 +43,25 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
static int smsc_phy_config_init(struct phy_device *phydev)
{
+ int __maybe_unused len;
+ struct device *dev __maybe_unused = &phydev->dev;
+ struct device_node *of_node __maybe_unused = dev->of_node;
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ int enable_energy = 1;
if (rc < 0)
return rc;
- /* Enable energy detect mode for this SMSC Transceivers */
- rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
- rc | MII_LAN83C185_EDPWRDOWN);
- if (rc < 0)
- return rc;
+ if (of_find_property(of_node, "smsc,disable-energy-detect", &len))
+ enable_energy = 0;
+
+ if (enable_energy) {
+ /* Enable energy detect mode for this SMSC Transceivers */
+ rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
+ rc | MII_LAN83C185_EDPWRDOWN);
+ if (rc < 0)
+ return rc;
+ }
return smsc_phy_ack_interrupt(phydev);
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] net, phy, smsc: add posibility to disable energy detect mode
On some boards the energy enable detect mode leads in trouble with some switches, so make the enabling of this mode configurable through DT. Therefore the property "smsc,disable-energy-detect" is introduced. Patch 1 introduces phy-handle support for the ti,cpsw driver. This is needed now for the smsc phy. Patch 2 adds the disable energy mode functionality to the smsc phy Changes in v2: - add comments from Florian Fainelli - I did not change disable property name into enable because I fear to break existing behaviour - add smsc vendor prefix - remove CONFIG_OF and use __maybe_unused - introduce "phy-handle" ability into ti,cpsw driver, so I can remove bogus: if (!of_node && dev->parent->of_node) of_node = dev->parent->of_node; construct. Therefore new patch for the ti,cpsw driver is necessary. Heiko Schocher (2): drivers: net: cpsw: add phy-handle parsing net: phy: smsc: disable energy detect mode Documentation/devicetree/bindings/net/cpsw.txt | 1 + .../devicetree/bindings/net/smsc-lan87xx.txt | 24 ++ drivers/net/ethernet/ti/cpsw.c | 15 ++ drivers/net/phy/smsc.c | 19 - 4 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: smsc: disable energy detect mode
Hello Florian, Am 16.10.2015 um 18:27 schrieb Florian Fainelli: 2015-10-13 21:17 GMT-07:00 Heiko Schocher : Hello Florian, Am 13.10.2015 um 21:26 schrieb Florian Fainelli: On 12/10/15 22:13, Heiko Schocher wrote: On some boards the energy enable detect mode leads in trouble with some switches, so make the enabling of this mode configurable through DT. Signed-off-by: Heiko Schocher --- .../devicetree/bindings/net/smsc-lan87xx.txt | 19 + drivers/net/phy/smsc.c | 24 +- 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt new file mode 100644 index 000..39aa1dc --- /dev/null +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt @@ -0,0 +1,19 @@ +SMSC LAN87xx Ethernet PHY + +Some boards require special tuning values. Configure them +through an Ethernet OF device node. + +Optional properties: + +- disable-energy-detect: + If set, do not enable energy detect mode for the SMSC phy. + default: enable energy detect mode Although energy detection is something that is implemented by many PHYs, I am not sure a generic property is suitable here, I would prefix that with the SMSC vendor prefix here to make it clear this only applies to this PHY. Hmm... but all PHYs should be able to enable, disable it in some way, or? It may not always be controlled directly at the PHY level, sometimes this is something that needs cooperation with the Ethernet MAC as well in case of integrated designs. Ah, ok! Would not you want to make it a reverse property here though, something like this: smsc,energy-detect: boolean, when present indicates the PHY reliably supports energy detection Yes, that was also my first thought, but currently, on this PHYs energy detect mode is on ... and if I introduce such a property, it will disable it for all existing boards, because property is missing ... so, maybe I break boards ... Fair enough, how about smsc,disabled-energy-detect or something like that then? Yes, changed it to "smsc,disable-energy-detect" bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: smsc: disable energy detect mode
Hello Florian,
Am 14.10.2015 um 06:17 schrieb Heiko Schocher:
Hello Florian,
Am 13.10.2015 um 21:26 schrieb Florian Fainelli:
On 12/10/15 22:13, Heiko Schocher wrote:
On some boards the energy enable detect mode leads in
trouble with some switches, so make the enabling of
this mode configurable through DT.
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/smsc-lan87xx.txt | 19 +
drivers/net/phy/smsc.c | 24 +-
2 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt
diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
new file mode 100644
index 000..39aa1dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -0,0 +1,19 @@
+SMSC LAN87xx Ethernet PHY
+
+Some boards require special tuning values. Configure them
+through an Ethernet OF device node.
+
+Optional properties:
+
+- disable-energy-detect:
+ If set, do not enable energy detect mode for the SMSC phy.
+ default: enable energy detect mode
Although energy detection is something that is implemented by many PHYs,
I am not sure a generic property is suitable here, I would prefix that
with the SMSC vendor prefix here to make it clear this only applies to
this PHY.
Hmm... but all PHYs should be able to enable, disable it in some way, or?
ping?
Would not you want to make it a reverse property here though, something
like this:
smsc,energy-detect: boolean, when present indicates the PHY reliably
supports energy detection
Yes, that was also my first thought, but currently, on this PHYs
energy detect mode is on ... and if I introduce such a property,
it will disable it for all existing boards, because property is
missing ... so, maybe I break boards ...
I have a v2 prepared, but what to do here?
+
+Examples:
+
+/* Attach to an Ethernet device with autodetected PHY */
+&cpsw_emac0 {
+phy_id = <&davinci_mdio>, <0>;
+phy-mode = "mii";
+disable-energy-detect;
+};
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 70b0895..f90fbf3 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
static int smsc_phy_config_init(struct phy_device *phydev)
{
+#ifdef CONFIG_OF
+int len;
+struct device *dev = &phydev->dev;
+struct device_node *of_node = dev->of_node;
That does not need to be ifdefd out, at best annontate with __maybe_unused?
Yes, I try it.
removed.
+#endif
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+int enable_energy = 1;
if (rc < 0)
return rc;
-/* Enable energy detect mode for this SMSC Transceivers */
-rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
- rc | MII_LAN83C185_EDPWRDOWN);
-if (rc < 0)
-return rc;
+#ifdef CONFIG_OF
+if (!of_node && dev->parent->of_node)
+of_node = dev->parent->of_node;
That looks strange, why would the property be placed at the parent level
when this is a PHY device tree node property?
Hmm.. I recheck this.
Hmm.. the drivers/net/ethernet/ti/cpsw.c has no phy-handle
support ... I added this, so my v2 is now a patchset of 2 patches.
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: phy: smsc: disable energy detect mode
Hello Florian,
Am 13.10.2015 um 21:26 schrieb Florian Fainelli:
On 12/10/15 22:13, Heiko Schocher wrote:
On some boards the energy enable detect mode leads in
trouble with some switches, so make the enabling of
this mode configurable through DT.
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/smsc-lan87xx.txt | 19 +
drivers/net/phy/smsc.c | 24 +-
2 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt
diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
new file mode 100644
index 000..39aa1dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -0,0 +1,19 @@
+SMSC LAN87xx Ethernet PHY
+
+Some boards require special tuning values. Configure them
+through an Ethernet OF device node.
+
+Optional properties:
+
+- disable-energy-detect:
+ If set, do not enable energy detect mode for the SMSC phy.
+ default: enable energy detect mode
Although energy detection is something that is implemented by many PHYs,
I am not sure a generic property is suitable here, I would prefix that
with the SMSC vendor prefix here to make it clear this only applies to
this PHY.
Hmm... but all PHYs should be able to enable, disable it in some way, or?
Would not you want to make it a reverse property here though, something
like this:
smsc,energy-detect: boolean, when present indicates the PHY reliably
supports energy detection
Yes, that was also my first thought, but currently, on this PHYs
energy detect mode is on ... and if I introduce such a property,
it will disable it for all existing boards, because property is
missing ... so, maybe I break boards ...
+
+Examples:
+
+ /* Attach to an Ethernet device with autodetected PHY */
+ &cpsw_emac0 {
+ phy_id = <&davinci_mdio>, <0>;
+ phy-mode = "mii";
+ disable-energy-detect;
+ };
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 70b0895..f90fbf3 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
static int smsc_phy_config_init(struct phy_device *phydev)
{
+#ifdef CONFIG_OF
+ int len;
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
That does not need to be ifdefd out, at best annontate with __maybe_unused?
Yes, I try it.
+#endif
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ int enable_energy = 1;
if (rc < 0)
return rc;
- /* Enable energy detect mode for this SMSC Transceivers */
- rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
- rc | MII_LAN83C185_EDPWRDOWN);
- if (rc < 0)
- return rc;
+#ifdef CONFIG_OF
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
That looks strange, why would the property be placed at the parent level
when this is a PHY device tree node property?
Hmm.. I recheck this.
+ if (of_find_property(of_node, "disable-energy-detect", &len))
+ enable_energy = 0;
+#endif
+ if (enable_energy) {
+ /* Enable energy detect mode for this SMSC Transceivers */
+ rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
+ rc | MII_LAN83C185_EDPWRDOWN);
+ if (rc < 0)
+ return rc;
+ }
return smsc_phy_ack_interrupt(phydev);
}
Thanks for your review.
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: phy: smsc: disable energy detect mode
On some boards the energy enable detect mode leads in
trouble with some switches, so make the enabling of
this mode configurable through DT.
Signed-off-by: Heiko Schocher
---
.../devicetree/bindings/net/smsc-lan87xx.txt | 19 +
drivers/net/phy/smsc.c | 24 +-
2 files changed, 38 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt
diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
new file mode 100644
index 000..39aa1dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt
@@ -0,0 +1,19 @@
+SMSC LAN87xx Ethernet PHY
+
+Some boards require special tuning values. Configure them
+through an Ethernet OF device node.
+
+Optional properties:
+
+- disable-energy-detect:
+ If set, do not enable energy detect mode for the SMSC phy.
+ default: enable energy detect mode
+
+Examples:
+
+ /* Attach to an Ethernet device with autodetected PHY */
+ &cpsw_emac0 {
+ phy_id = <&davinci_mdio>, <0>;
+ phy-mode = "mii";
+ disable-energy-detect;
+ };
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 70b0895..f90fbf3 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev)
static int smsc_phy_config_init(struct phy_device *phydev)
{
+#ifdef CONFIG_OF
+ int len;
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+#endif
int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS);
+ int enable_energy = 1;
if (rc < 0)
return rc;
- /* Enable energy detect mode for this SMSC Transceivers */
- rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
- rc | MII_LAN83C185_EDPWRDOWN);
- if (rc < 0)
- return rc;
+#ifdef CONFIG_OF
+ if (!of_node && dev->parent->of_node)
+ of_node = dev->parent->of_node;
+ if (of_find_property(of_node, "disable-energy-detect", &len))
+ enable_energy = 0;
+#endif
+ if (enable_energy) {
+ /* Enable energy detect mode for this SMSC Transceivers */
+ rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS,
+ rc | MII_LAN83C185_EDPWRDOWN);
+ if (rc < 0)
+ return rc;
+ }
return smsc_phy_ack_interrupt(phydev);
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
