[PATCH] net: fec: Rename "phy-reset-active-low" property

[Bernhard Walle]

>From the perspective of RESET, the meaning of the new property
is actually "active high". Thanks for Troy Kisky for pointing
that out.

Since the patch is in linux-next, this patch is incremental and doesn't
replace the original patch.

Signed-off-by: Bernhard Walle <bernh...@bwalle.de>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt | 2 +-
 drivers/net/ethernet/freescale/fec_main.c | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt 
b/Documentation/devicetree/bindings/net/fsl-fec.txt
index a4799ff..b037a9d 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -12,7 +12,7 @@ Optional properties:
   only if property "phy-reset-gpios" is available.  Missing the property
   will have the duration be 1 millisecond.  Numbers greater than 1000 are
   invalid and 1 millisecond will be used instead.
-- phy-reset-active-low : If present then the reset sequence using the GPIO
+- phy-reset-active-high : If present then the reset sequence using the GPIO
   specified in the "phy-reset-gpios" property is reversed (H=reset state,
   L=operation state).
 - phy-supply : regulator that powers the Ethernet PHY.
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index bad0ba2..37c0815 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3191,7 +3191,7 @@ static int fec_enet_init(struct net_device *ndev)
 static void fec_reset_phy(struct platform_device *pdev)
 {
int err, phy_reset;
-   bool active_low = false;
+   bool active_high = false;
int msec = 1;
struct device_node *np = pdev->dev.of_node;
 
@@ -3207,17 +3207,17 @@ static void fec_reset_phy(struct platform_device *pdev)
if (!gpio_is_valid(phy_reset))
return;
 
-   active_low = of_property_read_bool(np, "phy-reset-active-low");
+   active_high = of_property_read_bool(np, "phy-reset-active-high");
 
err = devm_gpio_request_one(>dev, phy_reset,
-   active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
+   active_high ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
"phy-reset");
if (err) {
dev_err(>dev, "failed to get phy-reset-gpios: %d\n", err);
return;
}
msleep(msec);
-   gpio_set_value_cansleep(phy_reset, !active_low);
+   gpio_set_value_cansleep(phy_reset, !active_high);
 }
 #else /* CONFIG_OF */
 static void fec_reset_phy(struct platform_device *pdev)
-- 
2.7.2

Re: [PATCH] net: fec: Add "phy-reset-active-low" property to DT

[Bernhard Walle]

Hi,

sorry for my late reply.

* Troy Kisky  [2016-03-01 18:16]:
>invalid and 1 millisecond will be used instead.
> +- phy-reset-active-low : If present then the reset sequence using the GPIO
> +  specified in the "phy-reset-gpios" property is reversed (H=reset state,
> +  L=operation state).
> 
> 
> 
> Shouldn't this be named phy-reset-active-high, as you are making the reset 
> active high
> H=reset, L= normal operation

Indeed, you're correct.

Should I send a new patch or a patch that corrects my first patch?
Because it already is in linux-next.

Regards,
Bernhard

[PATCH] net: fec: Add "phy-reset-active-low" property to DT

[Bernhard Walle]

We need that for a custom hardware that needs the reverse reset
sequence.

Signed-off-by: Bernhard Walle <bernh...@bwalle.de>
---
 Documentation/devicetree/bindings/net/fsl-fec.txt | 3 +++
 drivers/net/ethernet/freescale/fec_main.c | 8 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt 
b/Documentation/devicetree/bindings/net/fsl-fec.txt
index a9eb611..a4799ff 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -12,6 +12,9 @@ Optional properties:
   only if property "phy-reset-gpios" is available.  Missing the property
   will have the duration be 1 millisecond.  Numbers greater than 1000 are
   invalid and 1 millisecond will be used instead.
+- phy-reset-active-low : If present then the reset sequence using the GPIO
+  specified in the "phy-reset-gpios" property is reversed (H=reset state,
+  L=operation state).
 - phy-supply : regulator that powers the Ethernet PHY.
 - phy-handle : phandle to the PHY device connected to this device.
 - fixed-link : Assume a fixed link. See fixed-link.txt in the same directory.
diff --git a/drivers/net/ethernet/freescale/fec_main.c 
b/drivers/net/ethernet/freescale/fec_main.c
index 41c81f6..98caf87 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3229,6 +3229,7 @@ static int fec_enet_init(struct net_device *ndev)
 static void fec_reset_phy(struct platform_device *pdev)
 {
int err, phy_reset;
+   bool active_low = false;
int msec = 1;
struct device_node *np = pdev->dev.of_node;
 
@@ -3244,14 +3245,17 @@ static void fec_reset_phy(struct platform_device *pdev)
if (!gpio_is_valid(phy_reset))
return;
 
+   active_low = of_property_read_bool(np, "phy-reset-active-low");
+
err = devm_gpio_request_one(>dev, phy_reset,
-   GPIOF_OUT_INIT_LOW, "phy-reset");
+   active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
+   "phy-reset");
if (err) {
dev_err(>dev, "failed to get phy-reset-gpios: %d\n", err);
return;
}
msleep(msec);
-   gpio_set_value_cansleep(phy_reset, 1);
+   gpio_set_value_cansleep(phy_reset, !active_low);
 }
 #else /* CONFIG_OF */
 static void fec_reset_phy(struct platform_device *pdev)
-- 
2.7.1

Re: locking api self-test hanging

[Bernhard Walle]

* Andrew Morton [EMAIL PROTECTED] [2008-02-04 14:04]:
 
 but that code really needs help.

Using spin_lock_irqsave() is what rtc_get_rtc_time() does which was
used before I changed to get_rtc_time() does. So it should be ok. I
missed that difference. However, I agree with you the whole RTC
emulation per HPET is a bit unclean. However, I have too little
experience in this code area to come up with a proper redesign. I just
ported it from the old RTC to the new RTC module interface.

 Bernhard, I believe the checklist items in Documentation/SubmitChecklist
 would have prevented this at the source.

Yes. I must admit that I didn't know that document. I used
checkpatch.pl, but that's of course only for coding style. I'll try to
follow the points in SubmitChecklist in future.



Bernhard
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] proc_fs.h redux

[Bernhard Walle]

* Russell King [EMAIL PROTECTED] [2007-10-28 11:34]:
 
 If you go down that route, you end up with _lots_ of circular
 dependencies - header file X needs Y needs Z which needs X.  We've
 been there, several times.  It very quickly becomes quite
 unmaintainable - you end up with hard to predict behaviour from
 include files.
 
 The only realistic solution is to use forward declarations.

In header files, yes. But that's not true for implementation files.


Thanks,
   Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] proc_fs.h redux

[Bernhard Walle]

* Russell King [EMAIL PROTECTED] [2007-10-28 14:04]:
 On Sun, Oct 28, 2007 at 12:59:52PM +0100, Bernhard Walle wrote:
  * Russell King [EMAIL PROTECTED] [2007-10-28 11:34]:
   
   If you go down that route, you end up with _lots_ of circular
   dependencies - header file X needs Y needs Z which needs X.  We've
   been there, several times.  It very quickly becomes quite
   unmaintainable - you end up with hard to predict behaviour from
   include files.
   
   The only realistic solution is to use forward declarations.
  
  In header files, yes. But that's not true for implementation files.
 
 I don't think that needs saying - it's quite obvious.  You can't
 access the contents of structures without their definitions being
 available.

Of course. But there might be the case where an implementation file
doesn't access the structure itself but just passes the pointer to
some other function (which is implemented in another file). In that
case, you also have the choice between forward declaration and
including the header file in the implementation file.



Thanks,
   Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Fix config requirements of IPv6

[Bernhard Walle]

[PATCH] Fix config requirements of IPv6

net/ipv6/inet6_connection_sock.c in function __inet6_csk_dst_store() requires
flow_cache_genid which is defined in net/core/flow.c which is compiled when
CONFIG_XFRM is set.

This patch expresses that dependency.


Signed-off-by: Bernhard Walle [EMAIL PROTECTED]

---
 net/ipv6/Kconfig |1 +
 1 file changed, 1 insertion(+)

--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -5,6 +5,7 @@
 #   IPv6 as module will cause a CRASH if you try to unload it
 config IPV6
tristate The IPv6 protocol
+   select XFRM
default m
---help---
  This is complemental support for the IP version 6.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix crash in tg3 when using irqpoll

[Bernhard Walle]

Hello Michael,

* Michael Chan [EMAIL PROTECTED] [2007-03-22 23:04]:
 On Thu, 2007-03-22 at 21:46 +0100, Bernhard Walle wrote:
 
  This patch makes sure that even the tr32() instruction in the interrupt 
  handler
  is not executed which accesses PCI memory. Accessing PCI memory when
  pci_restore_state() is called is a bad idea because that function modifies
  the BARs of the PCI device.
 
 It is not caused by the BAR as it doesn't get changed in this case.  The
 pci_restore_state() call is to restore the memory enable bit in the PCI
 command register.  The tr32() call in tg3_interrupt() will cause a
 master abort if it is called before the memory enable bit has been
 restored.

Ok, thanks for the explanation. I wondered why you call
pci_restore_state() here, normally that's only called from .resume
handlers.

  --- mainline-msi-init.orig/drivers/net/tg3.c
  +++ mainline-msi-init/drivers/net/tg3.c
  @@ -3561,7 +3561,10 @@ static irqreturn_t tg3_interrupt(int irq
  struct net_device *dev = dev_id;
  struct tg3 *tp = netdev_priv(dev);
  struct tg3_hw_status *sblk = tp-hw_status;
  -   unsigned int handled = 1;
  +   unsigned int handled = 0;
  +
  +   if (tg3_irq_sync(tp))
  +   goto out;
 
 
 This will break other things.  When we disable interrupts, we set the
 irq_sync flag but allow one more interrupt to be generated.  The irq
 handler will simply mask off the interrupt when it sees the irq_sync
 flag.  With the above change, the irq handler can no longer mask off
 this trailing interrupt and you may get screaming interrupts as a
 result.

You're right, I had only the case in mind where the network device
doesn't generate any interrupts (in initialisation phase and in
shutdown phase) because it's disabled in the device, and the interrupt
handler is only called because of IRQ-sharing/irqpoll.

 Thanks for reporting this.  I'll try to come up with a good solution.

Could you please CC me, I'd like to test it here.


Thanks,
   Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Fix oops in xfrm4_dst_destroy()

[Bernhard Walle]

With 2.6.21-rc1, I get an oops when running 'ifdown eth0' and an IPsec
connection is active. If I shut down the connection before running 'ifdown
eth0', then there's no problem.  The critical operation of this script is to
kill dhcpd.

The problem is probably caused by commit with git identifier
4337226228e1cfc1d70ee975789c6bd070fb597c (Linus tree) [IPSEC]: IPv4 over IPv6
IPsec tunnel.

This patch fixes that oops. I don't know the network code of the Linux kernel
in deep, so if that fix is wrong, please change it. But please fix the oops. :)


Signed-off-by: Bernhard Walle [EMAIL PROTECTED]
---
 net/ipv4/xfrm4_policy.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.21-rc1/net/ipv4/xfrm4_policy.c
===
--- linux-2.6.21-rc1.orig/net/ipv4/xfrm4_policy.c
+++ linux-2.6.21-rc1/net/ipv4/xfrm4_policy.c
@@ -291,7 +291,7 @@ static void xfrm4_dst_destroy(struct dst
 
if (likely(xdst-u.rt.idev))
in_dev_put(xdst-u.rt.idev);
-   if (dst-xfrm-props.family == AF_INET  likely(xdst-u.rt.peer))
+   if (dst-xfrm  dst-xfrm-props.family == AF_INET  
likely(xdst-u.rt.peer))
inet_putpeer(xdst-u.rt.peer);
xfrm_dst_destroy(xdst);
 }
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: r8169: Crash after reloading driver if network hangs

[Bernhard Walle]

Hello,

* Francois Romieu [EMAIL PROTECTED] [2007-01-29 08:56]:
 Bernhard Walle [EMAIL PROTECTED] :
 
  @@ -1371,10 +1371,9 @@ static inline void rtl8169_request_timer
  return;
   
  init_timer(timer);
  -   timer-expires = jiffies + RTL8169_PHY_TIMEOUT;
  timer-data = (unsigned long)(dev);
  timer-function = rtl8169_phy_timer;
  -   add_timer(timer);
  +   mod_timer(timer, jiffies + RTL8169_PHY_TIMEOUT);
   }
  
  But I think _this_ change is unnecessary, ...
 
 add_timer() is not supposed to modify an existing timer whereas mod_timer()
 encompasses both 

that's clear.

 and the race exists in both direction as netif_running()
 is true as soon as dev_open() starts but way before dev-open() completes.

But rtl8169_request_timer() is only called from change_mtu() and
dev-open(). And, if you call init_timer(), you always have a new
timer, and the reference to an existing tp-timer will be lost.

 [...]
  ... but that looks good (better than my patch) and should resolve the issue,
  too. I can't test because it's triggered only if the network hangs and you
  know, the last one isn't reproducable.
 
 There will be something to test in the merge of realtek's stuff #2.

Great.


Regards,
Bernhard
-- 
SUSE LINUX Products GmbHE-Mail:  [EMAIL PROTECTED]
Maxfeldstr. 5   Phone:   +49 (911) 74053-0
90409 Nürnberg, Germany
OpenPGP DDAF6454: F61F 34CC 09CA FB82 C9F6  BA4B 8865 3696 DDAF 6454
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

r8169: Crash after reloading driver if network hangs

[Bernhard Walle]

Hello,

also after applying the latest patch that was posted on that mailing
list, I have still the problem described in
http://bugzilla.kernel.org/show_bug.cgi?id=5137.

So after a network hang with several 

NETDEV WATCHDOG: eth0: transmit timed out

in the kernel log, I removed the driver ('rmmod r8169') and wanted to
reload the driver ('modprobe r8169'). Now the system crashed:

kernel BUG at kernel/timer.c:407!
invalid opcode:  [1] SMP 
CPU 1 
Modules linked in: r8169 i915 drm deflate zlib_deflate twofish 
twofish_common serpent blowfi
sh des cbc ecb blkcipher aes xcbc sha256 w83627ehf hwmon i2c_isa sha1 ipv6 
md5 eeprom crypto
_null af_key snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device nfs lockd 
nfs_acl sunrpc af_pa
cket cpufreq_conservative cpufreq_ondemand cpufreq_userspace 
cpufreq_powersave acpi_cpufreq 
freq_table button battery ac loop rfcomm hidp l2cap hci_usb bluetooth 
dm_mod usb_storage fus
e snd_hda_intel snd_hda_codec snd_pcm iTCO_wdt ehci_hcd uhci_hcd intel_agp 
i2c_i801 i2c_core
 snd_timer snd soundcore snd_page_alloc iTCO_vendor_support usbcore floppy 
lp parport_pc ppd
ev parport ext3 mbcache jbd sg sr_mod cdrom edd fan ata_piix libata piix 
thermal processor s
d_mod scsi_mod ide_disk ide_core
Pid: 5724, comm: modprobe Not tainted 2.6.20-rc6-default #1
RIP: 0010:[80241dfe]  [80241dfe] mod_timer+0x7/0x22
RSP: 0018:8100566c9c20  EFLAGS: 00010246
RAX: 0006 RBX: 810061d8c000 RCX: 0001
RDX: 03e8 RSI: a58a RDI: 810061d8d1e0
RBP: 810061d8c4c0 R08: c2022000 R09: 810061d8c4c0
R10: 0046 R11: 0202 R12: 
R13: ff01 R14: 8100566c03e8 R15: 810061d8c000
FS:  2b38153566f0() GS:81007db9abc0() knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2: 557fc870 CR3: 540af000 CR4: 06e0
Process modprobe (pid: 5724, threadinfo 8100566c8000, task 
810037fde850)
Stack:  881b6d8b 0064 810061d8c4c0 c2022000
 881b88ef 00ff 00ff 
81ff6r8169: eth1: link up

 81ff 8111 81007d0079c8 0296
Call Trace:
 [881b6d8b] :r8169:rtl8169_set_speed+0x4b/0x53
 [881b88ef] :r8169:rtl8169_init_one+0x954/0x9c4
 [8031b440] pci_device_probe+0xe5/0x151
 [8036d620] really_probe+0x87/0x106
 [8036d83f] __driver_attach+0x6f/0xaf
 [8036d7d0] __driver_attach+0x0/0xaf
 [8036d7d0] __driver_attach+0x0/0xaf
 [8036cb86] bus_for_each_dev+0x43/0x6e
 [8036ce7c] bus_add_driver+0x6b/0x18d
 [8031b63b] __pci_register_driver+0x75/0xaa
 [80298300] sys_init_module+0x1793/0x1900
 [8025511e] system_call+0x7e/0x83


Code: 0f 0b eb fe 48 39 77 10 75 06 48 83 3f 00 75 05 e9 73 85 fd 
RIP  [80241dfe] mod_timer+0x7/0x22
 RSP 8100566c9c20

netif_running(dev) returns true although open() hasn't been called that sets
the function of the timer. dev-state is 6 (I checked that).

Simple fix is attached. Although that seems to fix the symptom and not
the cause, please apply it if you don't have a better solution.


---
 r8169.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/net/r8169.c   2007-01-28 18:19:56.0 +0100
+++ b/drivers/net/r8169.c   2007-01-28 18:22:50.0 +0100
@@ -810,7 +810,8 @@ static int rtl8169_set_speed(struct net_
 
ret = tp-set_speed(dev, autoneg, speed, duplex);
 
-   if (netif_running(dev)  (tp-phy_1000_ctrl_reg  ADVERTISE_1000FULL))
+   if (netif_running(dev)  tp-timer.function 
+   (tp-phy_1000_ctrl_reg  ADVERTISE_1000FULL))
mod_timer(tp-timer, jiffies + RTL8169_PHY_TIMEOUT);
 
return ret;


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: r8169: Crash after reloading driver if network hangs

[Bernhard Walle]

* Francois Romieu [EMAIL PROTECTED] [2007-01-28 20:04]:
 Bernhard Walle [EMAIL PROTECTED] :
 [...]
  Simple fix is attached. Although that seems to fix the symptom and not
  the cause, please apply it if you don't have a better solution.
 
 What about the patch below ?
 
 diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
 index 577babd..4e22af7 100644
 --- a/drivers/net/r8169.c
 +++ b/drivers/net/r8169.c
 @@ -1373,7 +1373,7 @@ static inline void rtl8169_request_timer(struct 
 net_device *dev)
   timer-expires = jiffies + RTL8169_PHY_TIMEOUT;
   timer-data = (unsigned long)(dev);
   timer-function = rtl8169_phy_timer;
 - add_timer(timer);
 + mod_timer(timer);
  }

Doesn't compile, I think you mean this?

@@ -1371,10 +1371,9 @@ static inline void rtl8169_request_timer
return;
 
init_timer(timer);
-   timer-expires = jiffies + RTL8169_PHY_TIMEOUT;
timer-data = (unsigned long)(dev);
timer-function = rtl8169_phy_timer;
-   add_timer(timer);
+   mod_timer(timer, jiffies + RTL8169_PHY_TIMEOUT);
 }

But I think _this_ change is unnecessary, ...

  #ifdef CONFIG_NET_POLL_CONTROLLER
 @@ -1448,7 +1448,7 @@ static void rtl8169_init_phy(struct net_device *dev, 
 struct rtl8169_private *tp)
  
   rtl8169_phy_reset(dev, tp);
  
 - rtl8169_set_speed(dev, autoneg, speed, duplex);
 + tp-set_speed(dev, autoneg, speed, duplex);
  
   if ((RTL_R8(PHYstatus)  TBI_Enable)  netif_msg_link(tp))
   printk(KERN_INFO PFX %s: TBI auto-negotiating\n, dev-name);

... but that looks good (better than my patch) and should resolve the issue,
too. I can't test because it's triggered only if the network hangs and you
know, the last one isn't reproducable.



Regards,
Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [rft] r8169: merge release 6.001.00 of Realtek's driver - take #1

[Bernhard Walle]

* Francois Romieu [EMAIL PROTECTED] [2007-01-25 00:17]:
 Untested, straight from the release early dept. You have been warned.

Works here.


Regards,
Bernhard
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html