Netdev List
 help / color / mirror / Atom feed
* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Sedat Dilek @ 2014-11-14 10:56 UTC (permalink / raw)
  To: Dan Williams
  Cc: Greg KH, David S. Miller,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Aleksander Morgado
In-Reply-To: <CA+icZUW2iRkoAK10ayJ9PFuFGWBQzKdFe+8mfd=8oKkAp1txtg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> On Tue, 2014-11-04 at 16:11 +0100, Sedat Dilek wrote:
>>> Hi,
>>>
>>> I wanted to understand what is going on the kernel-side when
>>> connecting to the Internet via a Huawei E173 USB web-stick (3rd
>>> Generation: UMTS / HSPA).
>>>
>>> Especially the correlation between the diverse USB/NET kernel-drivers
>>> and how the networking is setup.
>>
>
> [ Sitting in front of a foreign Windows machine ]
>
> [ CC Aleksander ]
>
> Hi Dan,
>
> sorry for the late (and short) response.
>
> AFAICS you have given a "skeleton" for a "usb-wwan-networking"
> documentation :-).
>
> Personally, I would like to take into account some kernel-config
> options and some more things.
>

I started with documenting...

I have still some difficulties in understanding USB WWAN Networking.
So, this is what I revealed...

##### USB: HUAWEI E173 3G/UMTS/HSPA INTERNET STICK

### USB-NETWORKING AND WWAN SETUP
CONFIG_USB_USBNET=m        <--- usb networking
CONFIG_USB_NET_CDCETHER=m  <--- usb-wwan (net) configuration
CONFIG_USB_SERIAL_WWAN=m   <--- usb-wwan (serial) configuration
CONFIG_USB_SERIAL_OPTION=m <--- usb-serial driver called "option"

### PPP OPTIONS
CONFIG_PPP=y
CONFIG_PPP_BSDCOMP=m
CONFIG_PPP_DEFLATE=m
CONFIG_PPP_FILTER=y
CONFIG_PPP_MULTILINK=y
CONFIG_PPP_ASYNC=m

Beyond the PPP options, I wanted to understand what
CONFIG_USB_NET_CDCETHER does and why I need it.
Can someone help?

Thanks.

- Sedat -

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/Kconfig#n189

P.S.: cdc_ether Kconfig option and checking my logs

>From [1]...
...
config USB_NET_CDCETHER
tristate "CDC Ethernet support (smart devices such as cable modems)"
depends on USB_USBNET
default y
help
 This option supports devices conforming to the Communication Device
 Class (CDC) Ethernet Control Model, a specification that's easy to
 implement in device firmware.  The CDC specifications are available
 from <http://www.usb.org/>.

 CDC Ethernet is an implementation option for DOCSIS cable modems
 that support USB connectivity, used for non-Microsoft USB hosts.
 The Linux-USB CDC Ethernet Gadget driver is an open implementation.
   This driver should work with at least the following devices:

   * Dell Wireless 5530 HSPA
     * Ericsson PipeRider (all variants)
   * Ericsson Mobile Broadband Module (all variants)
     * Motorola (DM100 and SB4100)
     * Broadcom Cable Modem (reference design)
   * Toshiba (PCX1100U and F3507g/F3607gw)
   * ...

 This driver creates an interface named "ethX", where X depends on
 what other networking devices you have in use.  However, if the
 IEEE 802 "local assignment" bit is set in the address, a "usbX"
 name is used instead.
...

>From my logs...

$ dmesg | egrep -i 'option|wwan|ppp|3-1.2|huawei|gsm|modem'
[    0.000000] please try 'cgroup_disable=memory' option if you don't
want memory cgroups
[    0.549498] PPP generic driver version 2.4.2
[    1.299059] usb 3-1.2: new high-speed USB device number 3 using ehci-pci
[    1.394084] usb 3-1.2: New USB device found, idVendor=12d1, idProduct=1436
[    1.394095] usb 3-1.2: New USB device strings: Mfr=4, Product=3,
SerialNumber=0
[    1.394100] usb 3-1.2: Product: HUAWEI Mobile
[    1.394103] usb 3-1.2: Manufacturer: HUAWEI Technology
[    2.115424] usb-storage 3-1.2:1.0: USB Mass Storage device detected
[    2.125026] usb-storage 3-1.2:1.1: USB Mass Storage device detected
[    2.125607] usb-storage 3-1.2:1.2: USB Mass Storage device detected
[    2.125888] usb-storage 3-1.2:1.3: USB Mass Storage device detected
[    2.126187] usb-storage 3-1.2:1.4: USB Mass Storage device detected
[    2.126461] usb-storage 3-1.2:1.5: USB Mass Storage device detected
[    2.127098] scsi host11: usb-storage 3-1.2:1.5
[    2.129370] usb-storage 3-1.2:1.6: USB Mass Storage device detected
[    2.131685] scsi host12: usb-storage 3-1.2:1.6
[    3.127317] scsi 11:0:0:0: CD-ROM            HUAWEI   Mass Storage
   2.31 PQ: 0 ANSI: 2
[    3.137589] scsi 12:0:0:0: Direct-Access     HUAWEI   SD Storage
   2.31 PQ: 0 ANSI: 2
[   13.500302] cdc_ether 3-1.2:1.1 wwan0: register 'cdc_ether' at
usb-0000:00:1a.0-1.2, Mobile Broadband Network Device,
02:50:f3:00:00:00
[   14.160221] usbcore: registered new interface driver option
[   14.160820] usbserial: USB Serial support registered for GSM modem (1-port)
[   14.160940] option 3-1.2:1.0: GSM modem (1-port) converter detected
[   14.163032] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB0
[   14.163305] option 3-1.2:1.3: GSM modem (1-port) converter detected
[   14.163676] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB1
[   14.163742] option 3-1.2:1.4: GSM modem (1-port) converter detected
[   14.165227] usb 3-1.2: GSM modem (1-port) converter now attached to ttyUSB2
[   72.877065] PPP BSD Compression module registered
[   72.881701] PPP Deflate Compression module registered
- EOT -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: net-next panic in ovs call to arch_fast_hash2 since e5a2c899
From: Hannes Frederic Sowa @ 2014-11-14 11:10 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, netdev, discuss, pshelar, ogerlitz
In-Reply-To: <12872.1415941444@famine>

Hi Jay,

On Do, 2014-11-13 at 21:04 -0800, Jay Vosburgh wrote:
> 	[ adding Hannes to Cc, which I should've done initially ]
> 
> David Miller <davem@davemloft.net> wrote:
> 
> >From: Jay Vosburgh <jay.vosburgh@canonical.com>
> >Date: Thu, 13 Nov 2014 18:15:32 -0800
> >
> >> 	The "have feature" function, __intel_crc4_2_hash2, does not
> >> clobber %r8, and so the call does not panic on a system with
> >> X86_FEATURE_XMM4_2, although I'm not sure if that's a deliberate
> >> compiler action or just happenstance because __intel_crc4_2_hash2 uses
> >> fewer registers than __jhash2.
> >
> >Perhaps alternative calls can only be used with assembler routines
> >that use specific calling conventions, and they therefore generally
> >don't work with C functions?
> 
> 	I don't know the answer to that, but a quick search suggests
> that arch_fast_hash and arch_fast_hash2 (both added by commit e5a2c899)
> may be the only cases of alternative calls that aren't supplying either
> single instructions or assembly language functions.
> 
> 	From looking at how the alternative calls are implemented (code
> patching at boot or module load time from a table stored in a special
> section of the object file), I'm skeptical that the compiler could know
> what's the right thing to do.
> 
> 	Hannes, can you shed any light on this?

Unfortunately I only tested this code with rhashtable, which itself
takes a function pointer to arch_fast_hash and thus doesn't need to care
about clobbering so much. As a first step, I am currently testing this
patch as a first step and check thoroughly. Thanks for the report:

diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..1b32c82 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -4,45 +4,12 @@
 #include <linux/cpufeature.h>
 #include <asm/alternative.h>
 
-u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed);
-u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed);
-
-/*
- * non-inline versions of jhash so gcc does not need to generate
- * duplicate code in every object file
- */
-u32 __jhash(const void *data, u32 len, u32 seed);
-u32 __jhash2(const u32 *data, u32 len, u32 seed);
-
 /*
  * for documentation of these functions please look into
  * <include/asm-generic/hash.h>
  */
 
-static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
-{
-       u32 hash;
-
-       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
-                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
-                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
-       return hash;
-}
-
-static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
-{
-       u32 hash;
-
-       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
-#ifdef CONFIG_X86_64
-                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
-#else
-                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
-#endif
-       return hash;
-}
+u32 arch_fast_hash(const void *data, u32 len, u32 seed);
+u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed);
 
 #endif /* __ASM_X86_HASH_H */
diff --git a/arch/x86/lib/hash.c b/arch/x86/lib/hash.c
index e143271..a0a7a7e 100644
--- a/arch/x86/lib/hash.c
+++ b/arch/x86/lib/hash.c
@@ -38,7 +38,7 @@
 #include <linux/hash.h>
 #include <linux/jhash.h>
 
-static inline u32 crc32_u32(u32 crc, u32 val)
+static u32 crc32_u32(u32 crc, u32 val)
 {
 #ifdef CONFIG_AS_CRC32
        asm ("crc32l %1,%0\n" : "+r" (crc) : "rm" (val));
@@ -71,7 +71,6 @@ u32 __intel_crc4_2_hash(const void *data, u32 len, u32 seed)
 
        return seed;
 }
-EXPORT_SYMBOL(__intel_crc4_2_hash);
 
 u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
 {
@@ -82,16 +81,45 @@ u32 __intel_crc4_2_hash2(const u32 *data, u32 len, u32 seed)
 
        return seed;
 }
-EXPORT_SYMBOL(__intel_crc4_2_hash2);
 
 u32 __jhash(const void *data, u32 len, u32 seed)
 {
        return jhash(data, len, seed);
 }
-EXPORT_SYMBOL(__jhash);
 
 u32 __jhash2(const u32 *data, u32 len, u32 seed)
 {
        return jhash2(data, len, seed);
 }
-EXPORT_SYMBOL(__jhash2);
+
+noinline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
+{
+       u32 hash;
+
+
+#ifdef CONFIG_X86_64
+       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+       alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+       return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash);
+
+noinline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
+{
+       u32 hash;
+
+
+#ifdef CONFIG_X86_64
+       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "D" (data), "S" (len), "d" (seed));
+#else
+       alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+                        "=a" (hash), "a" (data), "d" (len), "c" (seed));
+#endif
+       return hash;
+}
+EXPORT_SYMBOL(arch_fast_hash2);

^ permalink raw reply related

* [PATCH net] ipv4: Fix incorrect error code when adding an unreachable route
From: Panu Matilainen @ 2014-11-14 11:14 UTC (permalink / raw)
  To: netdev

Trying to add an unreachable route incorrectly returns -ESRCH if
if custom FIB rules are present:

[root@localhost ~]# ip route add 74.125.31.199 dev eth0 via 1.2.3.4
RTNETLINK answers: Network is unreachable
[root@localhost ~]# ip rule add to 55.66.77.88 table 200
[root@localhost ~]# ip route add 74.125.31.199 dev eth0 via 1.2.3.4
RTNETLINK answers: No such process
[root@localhost ~]#

Commit 83886b6b636173b206f475929e58fac75c6f2446 ("[NET]: Change "not found"
return value for rule lookup") changed fib_rules_lookup()
to use -ESRCH as a "not found" code internally, but for user space it
should be translated into -ENETUNREACH. Handle the translation centrally in
ipv4-specific fib_lookup(), leaving the DECnet case alone.

On a related note, commit b7a71b51ee37d919e4098cd961d59a883fd272d8
("ipv4: removed redundant conditional") removed a similar translation from
ip_route_input_slow() prematurely AIUI.

Fixes: b7a71b51ee37 ("ipv4: removed redundant conditional")
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 net/ipv4/fib_rules.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index f2e1573..8f7bd56 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -62,6 +62,10 @@ int __fib_lookup(struct net *net, struct flowi4 *flp, struct fib_result *res)
 	else
 		res->tclassid = 0;
 #endif
+
+	if (err == -ESRCH)
+		err = -ENETUNREACH;
+
 	return err;
 }
 EXPORT_SYMBOL_GPL(__fib_lookup);
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
From: Johan Hovold @ 2014-11-14 11:21 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Johan Hovold, Mark Rutland, Florian Fainelli, David S. Miller,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <20141113080927.GP30369@pengutronix.de>

On Thu, Nov 13, 2014 at 09:09:27AM +0100, Sascha Hauer wrote:
> On Wed, Nov 12, 2014 at 10:19:20AM +0100, Johan Hovold wrote:
> > On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:
> > > On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:
> > > > On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:
> > > > > On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:
> > > > > > Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding
> > > > > > documentation.
> > > > > > 
> > > > > > Cc: devicetree@vger.kernel.org
> > > > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/net/micrel.txt | 5 +++++
> > > > > >  1 file changed, 5 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > index a1bab5eaae02..9b08dd6551dd 100644
> > > > > > --- a/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > +++ b/Documentation/devicetree/bindings/net/micrel.txt
> > > > > > @@ -19,6 +19,11 @@ Optional properties:
> > > > > >  
> > > > > >                See the respective PHY datasheet for the mode values.
> > > > > >  
> > > > > > + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode
> > > > > > +
> > > > > > +		Whether 25 MHz (rather than 50 Mhz) clock mode is selected
> > > > > > +		when the rmii_ref_clk_sel bit is set.
> > > > > 
> > > > > s/_/-/ in property names please.
> > > > 
> > > > Ouch, copied from variable name, sorry.
> > > > 
> > > > > That said, I don't follow the meaning. Does this cause the kernel to do
> > > > > something different, or is is simply that a 25MHz ref clock is wired up?
> > > > 
> > > > Yes, the driver currently sets this configuration bit based on a common
> > > > clock binding.
> > > > 
> > > > However, it turns out the meaning of the bit is reversed on some PHY
> > > > variants. On most PHYs 50 MHz mode is selected by setting this bit,
> > > > whereas on the PHYs that need this new property, setting it selects 25
> > > > MHz mode instead.
> > > 
> > > Maybe rename the property to something like rmii-ref-clk-25mhz-active-high
> > > then? Also you should probably make it more explicit that this is a
> > > hardware property and not for adjusting the clock.
> > 
> > You're right, but how about calling it
> > 
> > 	micrel,rmii-reference-clock-select-inverted
> > 
> > Then no one will set it believing it will change the clock mode and
> > without reading the binding doc first.
> > 
> > The description could then read something like
> > 
> > 	micrel,rmii-reference-clock-select-inverted: RMII Reference
> > 		Clock Select bit is inverted
> > 
> > 	The RMII Reference Clock Select bit is inverted so that setting
> > 	it selects 25 MHz rather than 50 MHz clock mode. 
> > 
> > 	Note that this is only needed for PHY variants that has this bit
> > 	inverted and that a clock reference ("rmii-ref" below) is always
> > 	needed to select the actual mode.
> 
> "Inverted" only has a meaning when everybody agrees what's the normal
> case. Since that not the case I really prefer talking about
> "active-high" or "active-low".

Yes, but I'm reluctant to using "active-high" and "active-low" as this
is not a signal (or IO pin) we're dealing with.

It's a configuration bit, which (if set) selects 25 MHz mode. Hence I
still think something like 

	micrel,rmii_ref_clk_sel_25_mhz

or

	micrel,rmii_reference_clock_select_selects_25_mhz

is preferred. The binding documentation will still make it clear that a
clocks reference is also needed.

Johan

^ permalink raw reply

* Re: [PATCH v4 8/8] net: can: c_can: Add support for TI am3352 DCAN
From: Wolfram Sang @ 2014-11-14 11:26 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian,
	balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can,
	netdev
In-Reply-To: <5465CAA3.5030301@pengutronix.de>

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

On Fri, Nov 14, 2014 at 10:25:55AM +0100, Marc Kleine-Budde wrote:
> On 11/13/2014 05:06 PM, Wolfram Sang wrote:
> > On Fri, Nov 07, 2014 at 04:49:22PM +0200, Roger Quadros wrote:
> >> AM3352 SoC has 2 DCAN modules. Add compatible id and
> >> raminit driver data for am3352 DCAN.
> >>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> > 
> > Acked-by: Wolfram Sang <wsa@the-dreams.de>
> 
> For the whole series or just this patch?

Just this one.


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

^ permalink raw reply

* Re: [PATCH 0/2] xfrm: Do not hash socket policies
From: Steffen Klassert @ 2014-11-14 11:36 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20141113090811.GA3280@gondor.apana.org.au>

On Thu, Nov 13, 2014 at 05:08:11PM +0800, Herbert Xu wrote:
> On Thu, Nov 13, 2014 at 05:00:48PM +0800, Herbert Xu wrote:
> > Hi Steffen:
> > 
> > I'm working on converting the xfrm policy hashing over to RCU
> > due to performance complaints.  While doing this I noticed that
> > we're needlessly hashing socket policies.
> > 
> > Here are two patches to get rid of that and slightly clean things
> > up.
> 
> Oops that didn't work very well as I left out some rather important
> bits :)
> 
> Here is a second revision.

Both applied to ipsec-next, thanks Herbert!

^ permalink raw reply

* Re: [PATCH v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Hannes Frederic Sowa @ 2014-11-14 12:11 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S. Miller, Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Eric Dumazet, linux-api-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1415929010-9361-2-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>

On Do, 2014-11-13 at 17:36 -0800, Alexei Starovoitov wrote:
> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
> either update existing map element or create a new one.
> Initially the plan was to add a new command to handle the case of
> 'create new element if it didn't exist', but 'flags' style looks
> cleaner and overall diff is much smaller (more code reused), so add 'flags'
> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
>  #define BPF_ANY	0 /* create new element or update existing */
>  #define BPF_NOEXIST	1 /* create new element if it didn't exist */
>  #define BPF_EXIST	2 /* update existing element */

Would a cmpxchg-alike function be handy here?

Bye,
Hannes

^ permalink raw reply

* Re: linux-next: ath9k: build failure, ath_cmn_process_fft() redefinition
From: Oleksij Rempel @ 2014-11-14 12:30 UTC (permalink / raw)
  To: Jeremiah Mahler, linux-kernel, ath9k-devel, linville,
	linux-wireless, ath9k-devel, netdev
In-Reply-To: <20141114000747.GA6865@hudson.localdomain>


[-- Attachment #1.1: Type: text/plain, Size: 5068 bytes --]

Thank you,

fixes are queued for review.

Am 14.11.2014 um 01:07 schrieb Jeremiah Mahler:
> 
> In version 20141113 of the linux-next kernel, if it is compiled with
> CONFIG_ATH9K_DEBUGFS unset, an error about ath_cmn_process_fft() being
> redefined will be produced.
> 
>   make
>   ...
>     LD [M]  drivers/net/wireless/ath/ath9k/ath9k_hw.o
>     CC [M]  drivers/net/wireless/ath/ath9k/common-spectral.o
>     CC      lib/debug_locks.o
>     CC      lib/random32.o
>   drivers/net/wireless/ath/ath9k/common-spectral.c:40:5: error:
>   redefinition of ‘ath_cmn_process_fft’
>    int ath_cmn_process_fft(struct ath_spec_scan_priv *spec_priv, struct
>   ieee80211_hdr *hdr,
>        ^
>   In file included from drivers/net/wireless/ath/ath9k/common.h:27:0,
>                    from drivers/net/wireless/ath/ath9k/ath9k.h:27,
>                    from
>   drivers/net/wireless/ath/ath9k/common-spectral.c:18:
>   drivers/net/wireless/ath/ath9k/common-spectral.h:146:19: note: previous
>   definition of ‘ath_cmn_process_fft’ was here
>    static inline int ath_cmn_process_fft(struct ath_spec_scan_priv
>   *spec_priv,
>                      ^
>   scripts/Makefile.build:257: recipe for target
>   'drivers/net/wireless/ath/ath9k/common-spectral.o' failed
>   make[5]: *** [drivers/net/wireless/ath/ath9k/common-spectral.o] Error 1
>   scripts/Makefile.build:402: recipe for target
>   'drivers/net/wireless/ath/ath9k' failed
>   make[4]: *** [drivers/net/wireless/ath/ath9k] Error 2
>   scripts/Makefile.build:402: recipe for target 'drivers/net/wireless/ath'
>   failed
>   make[3]: *** [drivers/net/wireless/ath] Error 2
>   scripts/Makefile.build:402: recipe for target 'drivers/net/wireless'
>   failed
>   make[2]: *** [drivers/net/wireless] Error 2
>   scripts/Makefile.build:402: recipe for target 'drivers/net' failed
>   make[1]: *** [drivers/net] Error 2
>   Makefile:953: recipe for target 'drivers' failed
>   make: *** [drivers] Error 2
>   make: *** Waiting for unfinished jobs....
>     CC      lib/bust_spinlocks.o
>   ...
> 
> Bisecting the kernel found that the following patch was the cause.
> 
>   commit 67dc74f15f147b9f88702de2952d2951e3e000ec
>   Author: Oleksij Rempel <linux@rempel-privat.de>
>   Date:   Thu Nov 6 08:53:30 2014 +0100
>   
>       ath9k: move spectral.* to common-spectral.*
>       
>       and rename exports from ath9k_spectral_* to ath9k_cmn_spectral_*
>       
>       Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
>       Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> This patch mostly consists of renaming functions and moving code but
> there was a functional change to the Makefile.
> 
> common-spectral.h uses CONFIG_ATH9K_DEBUGFS to conditionally provide a
> prototype of ath_cmn_process_fft() when set or to define it as a noop
> when it is unset.  The Makefile was changed so that CONFIG_ATH9K_DEBUGFS
> no longer applied to common-spectral and this will result in two
> definitions of ath_cmn_process_fft().
> 
>   > --- a/drivers/net/wireless/ath/ath9k/Makefile
>   > +++ b/drivers/net/wireless/ath/ath9k/Makefile
>   > @@ -16,8 +16,7 @@ ath9k-$(CONFIG_ATH9K_DFS_CERTIFIED) += dfs.o
>   >  ath9k-$(CONFIG_ATH9K_TX99) += tx99.o
>   >  ath9k-$(CONFIG_ATH9K_WOW) += wow.o
>   >
>   > -ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o \
>   > -                                spectral.o
>   > +ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o
>   >
>   >  ath9k-$(CONFIG_ATH9K_STATION_STATISTICS) += debug_sta.o
>   >
>   > @@ -59,7 +58,8 @@ obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
>   >  ath9k_common-y:=       common.o \
>   >                         common-init.o \
>   >                         common-beacon.o \
>   > -                       common-debug.o
>   > +                       common-debug.o \
>   > +                       common-spectral.o
> 
> Reverting the patch solves one error, but then a new one is produced.
> 
>   make
>   ...
>     MODPOST 185 modules
>     CC      arch/x86/boot/edd.o
>     VOFFSET arch/x86/boot/voffset.h
>   ERROR: "ath9k_cmn_spectral_scan_trigger"
>   [drivers/net/wireless/ath/ath9k/ath9k.ko] undefined!
>   scripts/Makefile.modpost:90: recipe for target '__modpost' failed
>   make[1]: *** [__modpost] Error 1
>   ...
> 
> This error is caused by the patch before it.
> 
>   commit f00a422cc81ef665f5098c0bc43cb0c616e55a9b
>   Author: Oleksij Rempel <linux@rempel-privat.de>
>   Date:   Thu Nov 6 08:53:29 2014 +0100
>   
>       ath9k: move ath9k_spectral_scan_ from main.c to spectral.c
>       
>       Now we should be ready to make this code common.
>       
>       Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
>       Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> Since the code was moved from main.c to spectral.c, it is now involved
> with CONFIG_ATH9K_DEBUGFS, which causes it to break.
> 
> Reverting both the above patches resolves the build errors.
> 


-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

^ permalink raw reply

* Re: [PATCH v5 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-14 13:43 UTC (permalink / raw)
  To: Marc Kleine-Budde, wg, Linus Walleij
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <5464D661.7080505@pengutronix.de>

On 11/13/2014 06:03 PM, Marc Kleine-Budde wrote:
> On 11/13/2014 04:23 PM, Roger Quadros wrote:
>> DRA7 CAN IP suffers from a problem which causes it to be prevented
>> from fully turning OFF (i.e. stuck in transition) if the module was
>> disabled while there was traffic on the CAN_RX line.
>>
>> To work around this issue we select the SLEEP pin state by default
>> on probe and use the DEFAULT pin state on CAN up and back to the
>> SLEEP pin state on CAN down.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/net/can/c_can/c_can.c | 37 +++++++++++++++++++++++++++++++++++++
>>  drivers/net/can/c_can/c_can.h |  1 +
>>  2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
>> index 8e78bb4..c80cb3d 100644
>> --- a/drivers/net/can/c_can/c_can.c
>> +++ b/drivers/net/can/c_can/c_can.c
>> @@ -35,6 +35,7 @@
>>  #include <linux/list.h>
>>  #include <linux/io.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/pinctrl/consumer.h>
>>  
>>  #include <linux/can.h>
>>  #include <linux/can/dev.h>
>> @@ -587,6 +588,21 @@ static int c_can_chip_config(struct net_device *dev)
>>  	return c_can_set_bittiming(dev);
>>  }
>>  
>> +/*
>> + * Selects the pinctrl state specified in the name.
>> + */
>> +static void c_can_pinctrl_select_state(struct c_can_priv *priv,
>> +				      const char *name)
>> +{
>> +	if (!IS_ERR(priv->pinctrl)) {
>> +		struct pinctrl_state *s;
>> +
>> +		s = pinctrl_lookup_state(priv->pinctrl, name);
>> +		if (!IS_ERR(s))
>> +			pinctrl_select_state(priv->pinctrl, s);
>> +	}
>> +}
>> +
>>  static int c_can_start(struct net_device *dev)
>>  {
>>  	struct c_can_priv *priv = netdev_priv(dev);
>> @@ -603,6 +619,8 @@ static int c_can_start(struct net_device *dev)
>>  
>>  	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>  
>> +	/* activate pins */
>> +	c_can_pinctrl_select_state(priv, PINCTRL_STATE_DEFAULT);
>>  	return 0;
>>  }
>>  
>> @@ -611,6 +629,9 @@ static void c_can_stop(struct net_device *dev)
>>  	struct c_can_priv *priv = netdev_priv(dev);
>>  
>>  	c_can_irq_control(priv, false);
>> +
>> +	/* deactivate pins */
>> +	c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP);
>>  	priv->can.state = CAN_STATE_STOPPED;
>>  }
>>  
>> @@ -1244,6 +1265,22 @@ int register_c_can_dev(struct net_device *dev)
>>  	struct c_can_priv *priv = netdev_priv(dev);
>>  	int err;
>>  
>> +	priv->pinctrl = devm_pinctrl_get(dev->dev.parent);
> 
> What's dev->dev.parent?
> Ah, this is set by SET_NETDEV_DEV(dev, &pdev->dev); Good work!
> 
> I thought we had to set priv->pinctrl in the platform.c.
> 
>> +	if (!IS_ERR(priv->pinctrl)) {
>> +		struct pinctrl_state *s;
>> +
>> +		/* Deactivate pins to prevent DRA7 DCAN IP from being
>> +		 * stuck in transition when module is disabled.
>> +		 * Pins are activated in c_can_start() and deactivated
>> +		 * in c_can_stop()
>> +		 */
>> +		s = pinctrl_lookup_state(priv->pinctrl, PINCTRL_STATE_SLEEP);
>> +		if (!IS_ERR(s))
>> +			pinctrl_select_state(priv->pinctrl, s);
>> +	} else {
>> +		netdev_dbg(dev, "failed to get pinctrl\n");
>> +	}
>> +
> The above can be replace by this?
> 
> 	c_can_pinctrl_select_state(priv, PINCTRL_STATE_SLEEP)

Yes, indeed.

> 
> Then we don't have the worrying looking error message if there isn't any
> pinctrl, e.g. for the PCI driver with pinctrl enabled in the Kernel.
> 
> I just stumbled over pinctrl_pm_select_sleep_state(), is it possible to
> integrate this into runtime pm?
> 
> http://lxr.free-electrons.com/source/drivers/pinctrl/core.c#L1282

I think those functions are there for the same reason but not sure why aren't 
they used in runtime pm core.

Linus W. any hints?

cheers,
-roger

> 
>>  	c_can_pm_runtime_enable(priv);
>>  
>>  	dev->flags |= IFF_ECHO;	/* we support local echo */
>> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
>> index c6715ca..3cedf48 100644
>> --- a/drivers/net/can/c_can/c_can.h
>> +++ b/drivers/net/can/c_can/c_can.h
>> @@ -210,6 +210,7 @@ struct c_can_priv {
>>  	u32 comm_rcv_high;
>>  	u32 rxmasked;
>>  	u32 dlc[C_CAN_MSG_OBJ_TX_NUM];
>> +	struct pinctrl *pinctrl;
>>  };
>>  
>>  struct net_device *alloc_c_can_dev(void);
>>
> 
> Marc
> 


^ permalink raw reply

* [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 14:06 UTC (permalink / raw)
  To: netdev; +Cc: ogerlitz, pshelar, jesse, jay.vosburgh, discuss

In case the arch_fast_hash call gets inlined we need to tell gcc which
registers are clobbered with. Most callers where fine, as rhashtable
used arch_fast_hash via function pointer and thus the compiler took care
of that. In case of openvswitch the call got inlined and arch_fast_hash
touched registeres which gcc didn't know about.

Also don't use conditional compilation inside arguments, as this confuses
sparse.

Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Cc: Pravin Shelar <pshelar@nicira.com>
Cc: Jesse Gross <jesse@nicira.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 arch/x86/include/asm/hash.h | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..771cee0 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -23,11 +23,14 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "cc", "memory");
 #endif
 	return hash;
 }
@@ -36,11 +39,14 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "cc", "memory");
 #endif
 	return hash;
 }
-- 
1.9.3

^ permalink raw reply related

* [PATCH net] reciprocal_div: objects with exported symbols should be obj-y rather than lib-y
From: Hannes Frederic Sowa @ 2014-11-14 14:16 UTC (permalink / raw)
  To: netdev; +Cc: jim.epost

Otherwise the exported symbols might be discarded because of no users
in vmlinux.

Reported-by: Jim Davis <jim.epost@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 lib/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index 04e53dd..4b9baa4 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -10,7 +10,7 @@ endif
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o timerqueue.o\
 	 idr.o int_sqrt.o extable.o \
-	 sha1.o md5.o irq_regs.o reciprocal_div.o argv_split.o \
+	 sha1.o md5.o irq_regs.o argv_split.o \
 	 proportions.o flex_proportions.o ratelimit.o show_mem.o \
 	 is_single_threaded.o plist.o decompress.o kobject_uevent.o \
 	 earlycpio.o
@@ -26,7 +26,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
 	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
 	 gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
 	 bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
-	 percpu-refcount.o percpu_ida.o rhashtable.o
+	 percpu-refcount.o percpu_ida.o rhashtable.o reciprocal_div.o
 obj-y += string_helpers.o
 obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
 obj-y += kstrtox.o
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next v2] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 14:40 UTC (permalink / raw)
  To: netdev; +Cc: ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <4086c7bc9f7f9e8e2de9656c9e27ef1e71bb6423.1415973706.git.hannes@stressinduktion.org>

In case the arch_fast_hash call gets inlined we need to tell gcc which
registers are clobbered with. rhashtable was fine, because it used
arch_fast_hash via function pointer and thus the compiler took care of
that. In case of openvswitch the call got inlined and arch_fast_hash
touched registeres which gcc didn't know about.

Also don't use conditional compilation inside arguments, as this confuses
sparse.

Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Cc: Pravin Shelar <pshelar@nicira.com>
Cc: Jesse Gross <jesse@nicira.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---

v2)
After studying gcc documentation again, it occured to me that I need to
specificy all input operands in the clobber section, too. Otherwise gcc
can expect that the inline assembler section won't modify the inputs,
which is not true.

Bye,
Hannes


 arch/x86/include/asm/hash.h | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..a25c45a 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -23,11 +23,15 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
+			   "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "edx", "ecx", "cc", "memory");
 #endif
 	return hash;
 }
@@ -36,11 +40,15 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
+			   "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "edx", "ecx", "cc", "memory");
 #endif
 	return hash;
 }
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1] drivers: net: cpsw: Fix TX_IN_SEL offset
From: John Ogness @ 2014-11-14 14:42 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mugunthanvnm, balbi, george.cherian, jhovold, mpa,
	bhutchings, zonque, tklauser, netdev

The TX_IN_SEL offset for the CPSW_PORT/TX_IN_CTL register was
incorrect. This caused the Dual MAC mode to never get set when
it should. It also caused possible unintentional setting of a
bit in the CPSW_PORT/TX_BLKS_REM register.

The purpose of setting the Dual MAC mode for this register is to:

    "... allow packets from both ethernet ports to be written into
     the FIFO without one port starving the other port."
					- AM335x ARM TRM

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/net/ethernet/ti/cpsw.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index d879448..c560f9a 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -129,9 +129,9 @@ do {								\
 #define CPSW_VLAN_AWARE		BIT(1)
 #define CPSW_ALE_VLAN_AWARE	1
 
-#define CPSW_FIFO_NORMAL_MODE		(0 << 15)
-#define CPSW_FIFO_DUAL_MAC_MODE		(1 << 15)
-#define CPSW_FIFO_RATE_LIMIT_MODE	(2 << 15)
+#define CPSW_FIFO_NORMAL_MODE		(0 << 16)
+#define CPSW_FIFO_DUAL_MAC_MODE		(1 << 16)
+#define CPSW_FIFO_RATE_LIMIT_MODE	(2 << 16)
 
 #define CPSW_INTPACEEN		(0x3f << 16)
 #define CPSW_INTPRESCALE_MASK	(0x7FF << 0)
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Eric Dumazet @ 2014-11-14 14:50 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: netdev, ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <4086c7bc9f7f9e8e2de9656c9e27ef1e71bb6423.1415973706.git.hannes@stressinduktion.org>

On Fri, 2014-11-14 at 15:06 +0100, Hannes Frederic Sowa wrote:
> In case the arch_fast_hash call gets inlined we need to tell gcc which
> registers are clobbered with. Most callers where fine, as rhashtable
> used arch_fast_hash via function pointer and thus the compiler took care
> of that. In case of openvswitch the call got inlined and arch_fast_hash
> touched registeres which gcc didn't know about.
> 
> Also don't use conditional compilation inside arguments, as this confuses
> sparse.
> 

Please add a 
Fixes: 12-sha1 ("patch title")

> Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> Cc: Pravin Shelar <pshelar@nicira.com>
> Cc: Jesse Gross <jesse@nicira.com>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
>  arch/x86/include/asm/hash.h | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
> index a881d78..771cee0 100644
> --- a/arch/x86/include/asm/hash.h
> +++ b/arch/x86/include/asm/hash.h
> @@ -23,11 +23,14 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
>  {
>  	u32 hash;
>  
> -	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
>  #ifdef CONFIG_X86_64
> -			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
> +	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> +			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
> +			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");
>  #else




> -			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
> +	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> +			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
> +			 : "cc", "memory");
>  #endif
>  	return hash;
>  }
> @@ -36,11 +39,14 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
>  {
>  	u32 hash;
>  
> -	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
>  #ifdef CONFIG_X86_64
> -			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
> +	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
> +			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
> +			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");


Thats a lot of clobbers.

Alternative would be to use an assembly trampoline to save/restore them
before calling __jhash2

__intel_crc4_2_hash2 can probably be written in assembly, it is quite
simple.

^ permalink raw reply

* Re: Device Tree Binding for Marvell DSA Switch on imx28 board over Mdio Interface
From: Oliver Graute @ 2014-11-14 14:52 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev
In-Reply-To: <CA+KjHfZSeM=CPxbpoTAjbC3S5EKw+M59hii3t2zY4HePmFGZYg@mail.gmail.com>

On Fri, Nov 14, 2014 at 8:39 AM, Oliver Graute <oliver.graute@gmail.com> wrote:
> On Thu, Nov 13, 2014 at 9:03 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 11/13/2014 07:15 AM, Oliver Graute wrote:
>>> Hello Florian,
>>>
>>> On Wed, Nov 12, 2014 at 8:19 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>> On 11/12/2014 05:07 AM, Oliver Graute wrote:
>>>>> Hello,
>>>>>
>>>>> how do I specify the DSA node and the MDIO node in the Device Tree
>>>>> Binding to integrate a Marvell 88e6071 switch with a imx28 board?
>>>>>
>>>>> On my board the Marvell switch 88e6071 is connected via phy1 (on a
>>>>> imx28 PCB) to phy5 on the Marvell switch (on a Switch PCB). All phys
>>>>> are connected via the same MDIO Bus.
>>>>>
>>>>> I enabled the Marvell DSA Support Driver, Gianfar Ethernet Driver and
>>>>> Freescale PQ MDIO Driver in the Kernel (I' am not sure if this is the
>>>>> right choice for imx28 fec ethernet controller is it?)
>>>>>
>>>
>>> I changed my DeviceTree according to your proposal. Now I got a ENODEV 19
>>> in dsa_of_probe. Because  of_find_device_by_node(ethernet) is returning 0.
>>> Is my ethernet setting still wrong?
>>
>> Is your ethernet driver also modular? If so, you will need it to be
>> loaded *before* dsa. of_find_device_by_node() also needs the ethernet
>> driver to be a platform_driver.
>
> No my Freescale FEC PHY driver is not a module. FEC is a imx28/arm
> platform driver or not?
>
> I loaded the DSA as a Kernel module to make sure that the DSA probing
> is happening when the switch is really on. I enable the SWITCH ON Pin
> on bootup with a systemd started script. Then I write some registers
> on the switch with a userspace mii tool. This manually writing of some
> switch registers works fine via the MII Bus using ioctl(SIOCGMIIPHY).
>
> But i would like to integrate the switch with a full dsa driver.
> currently its failing with dsa_of_probe returns=-19
>

the dsa_core driver is probing the mii_bus before eth0 and eth1 are
detected via the FEC Driver.

[   20.716253] !!!!!enter dsa_init_module!!!!!
[   20.777046] !!!!Enter dsa Probe!!!!!
[   20.803422] Distributed Switch Architecture driver version 0.1
[   20.809295] !!!!!Enter dsa_of_probe!!!!!
[   20.888268] !!!!!mdio->name=mdio mdio->type=mdio
mdio->full_name=/mdio@800f0040 !!!!!
[   20.999618] !!!!!np->name=dsa np->type=<NULL> np->full_name=/dsa@0 !!!!!
[   21.097805] !!!!before of_mdio_find_bus!!!!!
[   21.137278] !!!!!enter of_mdio_find_bus!!!!!
[   21.190232] !!!!!enter of_mdio_bus_match!!!!!
[   21.194635] !!!!!enter of_mdio_bus_match!!!!!
[   21.199000] !!!!!enter of_mdio_bus_match!!!!!
[   21.300627] !!!!Leave of_mdio_find_bus !!!!!
[   21.304949] !!!!after of_mdio_find_bus mdio_bus=Freescale
PowerQUICC MII Bus !!!!!
[   21.456904] !!!!before of_parse_phandle dsa,ethernet!!!!!
[   21.570569] !!!!before of find_device_by_node!!!!!
[   21.575416] !!!!!ethernet->name=ethernet ethernet->type=<NULL>
ethernet->full_name=/ahb@80080000/ethernet@800f4000 !!!!!
[   21.860234] !!!!! enter of_find_device_by_node !!!!!
[   21.865284] !!!!! Leave of_find_device_by_node dev=c790fe10 !!!!!
[   21.970600] !!!!! dev->init_name=(null) !!!!!
[   21.975001] before to_platform_device test->name=800f4000.ethernet
[   22.088915] !!!!before of kzalloc!!!!!
[   22.134753] !!!!before pd->netdev!!!!!
[   22.138548] !!!!before dev_to_net_device!!!!!
[   22.210241] !!!!dev_put(dev)!!!!!
[   22.213600] !!!!kzalloc!!!!!
[   22.216493] !!!!platform_set_drv_data!!!!!
[   22.313247] !!!!!enter dev_to_mii_bus!!!!!
[   22.317393] !!!!!enter dsa_switch_setup!!!!!
[   22.394756] !!!!name=!!!!!
[   22.397691] !!!!bus->name=Freescale PowerQUICC MII Bus!!!!!
[   22.502050] !!!!pd->sw_addr=3!!!!!
[   22.505489] !!!!Enter dsa_switch_probe!!!!!
[   22.509685] !!!!Leave dsa_switch_probe!!!!!
[   22.630239] eth1[0]: could not detect attached switch
[   22.635337] eth1[0]: couldn't create dsa switch instance (error -22)
[   22.740538] !!!!Leave dsa Probe!!!!!
[   22.794305] !!!!!leave dsa_init_module!!!!!

[   65.954070] fec 800f0000.ethernet eth0: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=800f0000.etherne:00, irq=-1)
[   66.067135] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[   66.532877] fec 800f4000.ethernet eth1: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=800f0000.etherne:01, irq=-1)

if i manually rmmod and modprobe the dsa_core driver  after FEC PHY
detection again i got a  EEXIST 17

modprobe dsa_core
[  212.770578] !!!!!enter dsa_init_module!!!!!
[  212.775121] !!!!Enter dsa Probe!!!!!
[  212.778726] Distributed Switch Architecture driver version 0.1
[  212.791071] !!!!!Enter dsa_of_probe!!!!!
[  212.795191] !!!!!mdio->name=mdio mdio->type=mdio
mdio->full_name=/mdio@800f0040 !!!!!
[  212.805452] !!!!!np->name=dsa np->type=<NULL> np->full_name=/dsa@0 !!!!!
[  212.813355] !!!!before of_mdio_find_bus!!!!!
[  212.817669] !!!!!enter of_mdio_find_bus!!!!!
[  212.823707] !!!!!enter of_mdio_bus_match!!!!!
[  212.828111] !!!!!enter of_mdio_bus_match!!!!!
[  212.834213] !!!!!enter of_mdio_bus_match!!!!!
[  212.838620] !!!!Leave of_mdio_find_bus !!!!!
[  212.844655] !!!!after of_mdio_find_bus mdio_bus=Freescale
PowerQUICC MII Bus !!!!!
[  212.853684] !!!!before of_parse_phandle dsa,ethernet!!!!!
[  212.859179] !!!!before of find_device_by_node!!!!!
[  212.866019] !!!!!ethernet->name=ethernet ethernet->type=<NULL>
ethernet->full_name=/ahb@80080000/ethernet@800f4000 !!!!!
[  212.878029] !!!!! enter of_find_device_by_node !!!!!
[  212.884159] !!!!! Leave of_find_device_by_node dev=c790fe10 !!!!!
[  212.891366] !!!!! dev->init_name=(null) !!!!!
[  212.895769]
[  212.895769] before to_platform_device test->name=800f4000.ethernet
[  212.905738] !!!!before of kzalloc!!!!!
[  212.909586] !!!!before pd->netdev!!!!!
[  212.915402] !!!!before dev_to_net_device!!!!!
[  212.919817] !!!!dev_put(dev)!!!!!
[  212.925431] dsa: probe of dsa.5 failed with error -17
[  212.936922] !!!!!leave dsa_init_module!!!!!


best regards,

Oliver

^ permalink raw reply

* Re: [PATCH v2] can: Fix bug in suspend/resume
From: Sören Brinkmann @ 2014-11-14 15:05 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
	linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
	Kedareswara rao Appana
In-Reply-To: <5465C34D.4030805@pengutronix.de>

On Fri, 2014-11-14 at 09:54AM +0100, Marc Kleine-Budde wrote:
> On 11/14/2014 09:16 AM, Kedareswara rao Appana wrote:
> > The drvdata in the suspend/resume is of type struct net_device,
> > not the platform device.Enable the clocks in the suspend before
> > accessing the registers of the CAN.
> > 
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> > Changes for v2:
> >   - Removed the struct platform_device* from suspend/resume
> >     as suggest by Lothar.
> >   - The clocks are getting disabled and un prepared at the end of the probe. 
> >     In the suspend the driver is doing a register write.In order
> >     To do that register write we have to again enable and prepare the clocks.
> 
> Please look the at suspend/resume code and count the
> clock_enable/disable manually. After a suspend/resume cycle, you have
> enabled the clock twice, but disabled it once.
> 
> I think you have to abstract the clock handling behind runtime PM. I
> haven't done this myself yet, but the strong feeling that this is a
> possible solution to your problem. These links might help:

I agree, the clock handling looks weird. Also the clk_disable calls in
xcan_get_berr_counter() look suspicious to me, but I might be wrong.
I think you can take a look at gpio-zynq for an example for runtime_pm
usage. I think the usage model in that driver is similar to here.

	Thanks,
	Sören

^ permalink raw reply

* Re: [PATCH v2] can: Fix bug in suspend/resume
From: Marc Kleine-Budde @ 2014-11-14 15:09 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
	linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
	Kedareswara rao Appana
In-Reply-To: <e5f5913b51a54222aa1bb0680a77e56d@BN1AFFO11FD055.protection.gbl>

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

On 11/14/2014 04:05 PM, Sören Brinkmann wrote:
> On Fri, 2014-11-14 at 09:54AM +0100, Marc Kleine-Budde wrote:
>> On 11/14/2014 09:16 AM, Kedareswara rao Appana wrote:
>>> The drvdata in the suspend/resume is of type struct net_device,
>>> not the platform device.Enable the clocks in the suspend before
>>> accessing the registers of the CAN.
>>>
>>> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
>>> ---
>>> Changes for v2:
>>>   - Removed the struct platform_device* from suspend/resume
>>>     as suggest by Lothar.
>>>   - The clocks are getting disabled and un prepared at the end of the probe. 
>>>     In the suspend the driver is doing a register write.In order
>>>     To do that register write we have to again enable and prepare the clocks.
>>
>> Please look the at suspend/resume code and count the
>> clock_enable/disable manually. After a suspend/resume cycle, you have
>> enabled the clock twice, but disabled it once.
>>
>> I think you have to abstract the clock handling behind runtime PM. I
>> haven't done this myself yet, but the strong feeling that this is a
>> possible solution to your problem. These links might help:
> 
> I agree, the clock handling looks weird. Also the clk_disable calls in
> xcan_get_berr_counter() look suspicious to me, but I might be wrong.
> I think you can take a look at gpio-zynq for an example for runtime_pm
> usage. I think the usage model in that driver is similar to here.

The xcan_get_berr_counter() function is correct, when doing manual (i.e.
non runtime-pm) clock handling. This function might be called if the
interface is down, this means clocks are disabled.

Marc


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 15:13 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <1415976656.17262.41.camel@edumazet-glaptop2.roam.corp.google.com>

On Fr, 2014-11-14 at 06:50 -0800, Eric Dumazet wrote:
> On Fri, 2014-11-14 at 15:06 +0100, Hannes Frederic Sowa wrote:
> > In case the arch_fast_hash call gets inlined we need to tell gcc which
> > registers are clobbered with. Most callers where fine, as rhashtable
> > used arch_fast_hash via function pointer and thus the compiler took care
> > of that. In case of openvswitch the call got inlined and arch_fast_hash
> > touched registeres which gcc didn't know about.
> > 
> > Also don't use conditional compilation inside arguments, as this confuses
> > sparse.
> > 
> 
> Please add a 
> Fixes: 12-sha1 ("patch title")

I forgot, will send new version with tag added.

> 
> > Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
> > Cc: Pravin Shelar <pshelar@nicira.com>
> > Cc: Jesse Gross <jesse@nicira.com>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > ---
> >  arch/x86/include/asm/hash.h | 18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
> > index a881d78..771cee0 100644
> > --- a/arch/x86/include/asm/hash.h
> > +++ b/arch/x86/include/asm/hash.h
> > @@ -23,11 +23,14 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
> >  {
> >  	u32 hash;
> >  
> > -	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> >  #ifdef CONFIG_X86_64
> > -			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
> > +	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> > +			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
> > +			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");
> >  #else
> 
> 
> 
> 
> > -			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
> > +	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
> > +			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
> > +			 : "cc", "memory");
> >  #endif
> >  	return hash;
> >  }
> > @@ -36,11 +39,14 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
> >  {
> >  	u32 hash;
> >  
> > -	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
> >  #ifdef CONFIG_X86_64
> > -			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
> > +	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
> > +			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
> > +			 : "rcx", "r8", "r9", "r10", "r11", "cc", "memory");
> 
> 
> Thats a lot of clobbers.

Yes, those are basically all callee-clobbered registers for the
particular architecture. I didn't look at the generated code for jhash
and crc_hash because I want this code to always be safe, independent of
the version and optimization levels of gcc.

> Alternative would be to use an assembly trampoline to save/restore them
> before calling __jhash2

This version provides the best hints on how to allocate registers to the
optimizers. E.g. it could avoid using callee-clobbered registers but use
callee-saved ones. If we build a trampoline, we need to save and reload
all registers all the time. This version just lets gcc decide how to do
that.

> __intel_crc4_2_hash2 can probably be written in assembly, it is quite
> simple.

Sure, but all the pre and postconditions must hold for both, jhash and
intel_crc4_2_hash and I don't want to rewrite jhash in assembler.

Thanks,
Hannes

^ permalink raw reply

* [PATCH net-next v3] fast_hash: clobber registers correctly for inline function use
From: Hannes Frederic Sowa @ 2014-11-14 15:17 UTC (permalink / raw)
  To: netdev; +Cc: ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <1415976656.17262.41.camel@edumazet-glaptop2.roam.corp.google.com>

In case the arch_fast_hash call gets inlined we need to tell gcc which
registers are clobbered with. rhashtable was fine, because it used
arch_fast_hash via function pointer and thus the compiler took care of
that. In case of openvswitch the call got inlined and arch_fast_hash
touched registeres which gcc didn't know about.

Also don't use conditional compilation inside arguments, as this confuses
sparse.

Fixes: e5a2c899957659c ("fast_hash: avoid indirect function calls")
Reported-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Cc: Pravin Shelar <pshelar@nicira.com>
Cc: Jesse Gross <jesse@nicira.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---


v2)
After studying gcc documentation again, it occured to me that I need to
specificy all input operands in the clobber section, too. Otherwise gcc
can expect that the inline assembler section won't modify the inputs,
which is not true.

v3)
added Fixes tag

Bye,
Hannes

 arch/x86/include/asm/hash.h | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/hash.h b/arch/x86/include/asm/hash.h
index a881d78..a25c45a 100644
--- a/arch/x86/include/asm/hash.h
+++ b/arch/x86/include/asm/hash.h
@@ -23,11 +23,15 @@ static inline u32 arch_fast_hash(const void *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
+			   "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash, __intel_crc4_2_hash, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "edx", "ecx", "cc", "memory");
 #endif
 	return hash;
 }
@@ -36,11 +40,15 @@ static inline u32 arch_fast_hash2(const u32 *data, u32 len, u32 seed)
 {
 	u32 hash;
 
-	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
 #ifdef CONFIG_X86_64
-			 "=a" (hash), "D" (data), "S" (len), "d" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "D" (data), "S" (len), "d" (seed)
+			 : "rdi", "rsi", "rdx", "rcx", "r8", "r9", "r10", "r11",
+			   "cc", "memory");
 #else
-			 "=a" (hash), "a" (data), "d" (len), "c" (seed));
+	alternative_call(__jhash2, __intel_crc4_2_hash2, X86_FEATURE_XMM4_2,
+			 "=a" (hash), "a" (data), "d" (len), "c" (seed)
+			 : "edx", "ecx", "cc", "memory");
 #endif
 	return hash;
 }
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH v2] can: Fix bug in suspend/resume
From: Sören Brinkmann @ 2014-11-14 15:20 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
	linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
	Kedareswara rao Appana
In-Reply-To: <54661B2F.2000704@pengutronix.de>

On Fri, 2014-11-14 at 04:09PM +0100, Marc Kleine-Budde wrote:
> On 11/14/2014 04:05 PM, Sören Brinkmann wrote:
> > On Fri, 2014-11-14 at 09:54AM +0100, Marc Kleine-Budde wrote:
> >> On 11/14/2014 09:16 AM, Kedareswara rao Appana wrote:
> >>> The drvdata in the suspend/resume is of type struct net_device,
> >>> not the platform device.Enable the clocks in the suspend before
> >>> accessing the registers of the CAN.
> >>>
> >>> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> >>> ---
> >>> Changes for v2:
> >>>   - Removed the struct platform_device* from suspend/resume
> >>>     as suggest by Lothar.
> >>>   - The clocks are getting disabled and un prepared at the end of the probe. 
> >>>     In the suspend the driver is doing a register write.In order
> >>>     To do that register write we have to again enable and prepare the clocks.
> >>
> >> Please look the at suspend/resume code and count the
> >> clock_enable/disable manually. After a suspend/resume cycle, you have
> >> enabled the clock twice, but disabled it once.
> >>
> >> I think you have to abstract the clock handling behind runtime PM. I
> >> haven't done this myself yet, but the strong feeling that this is a
> >> possible solution to your problem. These links might help:
> > 
> > I agree, the clock handling looks weird. Also the clk_disable calls in
> > xcan_get_berr_counter() look suspicious to me, but I might be wrong.
> > I think you can take a look at gpio-zynq for an example for runtime_pm
> > usage. I think the usage model in that driver is similar to here.
> 
> The xcan_get_berr_counter() function is correct, when doing manual (i.e.
> non runtime-pm) clock handling. This function might be called if the
> interface is down, this means clocks are disabled.

I see, thanks for the clarification. Guess that should become
pm_runtime_get_sync() and pm_runtime_put() when converting to
runtime_pm.

	Sören

^ permalink raw reply

* Re: [PATCH v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Alexei Starovoitov @ 2014-11-14 15:33 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David S. Miller, Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
	Eric Dumazet, Linux API, Network Development, LKML
In-Reply-To: <1415967071.15154.9.camel@localhost>

On Fri, Nov 14, 2014 at 4:11 AM, Hannes Frederic Sowa
<hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r@public.gmane.org> wrote:
> On Do, 2014-11-13 at 17:36 -0800, Alexei Starovoitov wrote:
>> the current meaning of BPF_MAP_UPDATE_ELEM syscall command is:
>> either update existing map element or create a new one.
>> Initially the plan was to add a new command to handle the case of
>> 'create new element if it didn't exist', but 'flags' style looks
>> cleaner and overall diff is much smaller (more code reused), so add 'flags'
>> attribute to BPF_MAP_UPDATE_ELEM command with the following meaning:
>>  #define BPF_ANY      0 /* create new element or update existing */
>>  #define BPF_NOEXIST  1 /* create new element if it didn't exist */
>>  #define BPF_EXIST    2 /* update existing element */
>
> Would a cmpxchg-alike function be handy here?

you mean cmpxchg command in addition to
update() command ?
May be... it will have an extra 'value' argument
(key, old_value, new_value)
I don't have a use case for it yet though.

^ permalink raw reply

* Re: [PATCH net-next] fast_hash: clobber registers correctly for inline function use
From: Eric Dumazet @ 2014-11-14 15:33 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: netdev, ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <1415978022.15154.31.camel@localhost>

On Fri, 2014-11-14 at 16:13 +0100, Hannes Frederic Sowa wrote:
> > 
> > 
> > Thats a lot of clobbers.
> 
> Yes, those are basically all callee-clobbered registers for the
> particular architecture. I didn't look at the generated code for jhash
> and crc_hash because I want this code to always be safe, independent of
> the version and optimization levels of gcc.
> 
> > Alternative would be to use an assembly trampoline to save/restore them
> > before calling __jhash2
> 
> This version provides the best hints on how to allocate registers to the
> optimizers. E.g. it could avoid using callee-clobbered registers but use
> callee-saved ones. If we build a trampoline, we need to save and reload
> all registers all the time. This version just lets gcc decide how to do
> that.
> 
> > __intel_crc4_2_hash2 can probably be written in assembly, it is quite
> > simple.
> 
> Sure, but all the pre and postconditions must hold for both, jhash and
> intel_crc4_2_hash and I don't want to rewrite jhash in assembler.

We write optimized code for current cpus.

With current generation of cpus, we have crc32 support.

The fallback having to save/restore few registers, we don't care, as the
fallback has huge cost anyway.

You don't have to write jhash() in assembler, you misunderstood me.

We only have to provide a trampoline in assembler, with maybe 10
instructions.

Then gcc will know that we do not clobber registers for the optimized
case.

^ permalink raw reply

* Re: [PATCH v2] can: Fix bug in suspend/resume
From: Marc Kleine-Budde @ 2014-11-14 15:35 UTC (permalink / raw)
  To: Sören Brinkmann
  Cc: Kedareswara rao Appana, wg, michal.simek, grant.likely, robh+dt,
	linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
	Kedareswara rao Appana
In-Reply-To: <8d29d376aa02429dabd754b505fb336d@BN1BFFO11FD041.protection.gbl>

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

On 11/14/2014 04:20 PM, Sören Brinkmann wrote:
>>>> Please look the at suspend/resume code and count the
>>>> clock_enable/disable manually. After a suspend/resume cycle, you have
>>>> enabled the clock twice, but disabled it once.
>>>>
>>>> I think you have to abstract the clock handling behind runtime PM. I
>>>> haven't done this myself yet, but the strong feeling that this is a
>>>> possible solution to your problem. These links might help:
>>>
>>> I agree, the clock handling looks weird. Also the clk_disable calls in
>>> xcan_get_berr_counter() look suspicious to me, but I might be wrong.
>>> I think you can take a look at gpio-zynq for an example for runtime_pm
>>> usage. I think the usage model in that driver is similar to here.
>>
>> The xcan_get_berr_counter() function is correct, when doing manual (i.e.
>> non runtime-pm) clock handling. This function might be called if the
>> interface is down, this means clocks are disabled.
> 
> I see, thanks for the clarification. Guess that should become
> pm_runtime_get_sync() and pm_runtime_put() when converting to
> runtime_pm.

Yes, as far as I understand runtime pm.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH v5 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Roger Quadros @ 2014-11-14 15:37 UTC (permalink / raw)
  To: wg, mkl
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <1415371762-29885-5-git-send-email-rogerq@ti.com>

Some TI SoCs like DRA7 have a RAMINIT register specification
different from the other AMxx SoCs and as expected by the
existing driver.

To add more insanity, this register is shared with other
IPs like DSS, PCIe and PWM.

Provides a more generic mechanism to specify the RAMINIT
register location and START/DONE bit position and use the
syscon/regmap framework to access the register.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 .../devicetree/bindings/net/can/c_can.txt          |   3 +
 drivers/net/can/c_can/c_can.h                      |  11 +-
 drivers/net/can/c_can/c_can_platform.c             | 113 ++++++++++++++-------
 3 files changed, 87 insertions(+), 40 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
index 8f1ae81..a3ca3ee 100644
--- a/Documentation/devicetree/bindings/net/can/c_can.txt
+++ b/Documentation/devicetree/bindings/net/can/c_can.txt
@@ -12,6 +12,9 @@ Required properties:
 Optional properties:
 - ti,hwmods		: Must be "d_can<n>" or "c_can<n>", n being the
 			  instance number
+- syscon-raminit	: Handle to system control region that contains the
+			  RAMINIT register, register offset to the RAMINIT
+			  register and the CAN instance number (0 offset).
 
 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/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 3c305a1..0e17c7b 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -179,6 +179,14 @@ struct c_can_driver_data {
 	bool raminit_pulse;	/* If set, sets and clears START bit (pulse) */
 };
 
+/* Out of band RAMINIT register access via syscon regmap */
+struct c_can_raminit {
+	struct regmap *syscon;	/* for raminit ctrl. reg. access */
+	unsigned int reg;	/* register index within syscon */
+	u8 start_bit;
+	u8 done_bit;
+};
+
 /* c_can private data structure */
 struct c_can_priv {
 	struct can_priv can;	/* must be the first member */
@@ -196,8 +204,7 @@ struct c_can_priv {
 	const u16 *regs;
 	void *priv;		/* for board-specific data */
 	enum c_can_dev_id type;
-	u32 __iomem *raminit_ctrlreg;
-	int instance;
+	struct c_can_raminit raminit_sys;	/* RAMINIT via syscon regmap */
 	void (*raminit) (const struct c_can_priv *priv, bool enable);
 	u32 comm_rcv_high;
 	u32 rxmasked;
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 1546c2b..89739a1 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,14 +32,13 @@
 #include <linux/clk.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <linux/can/dev.h>
 
 #include "c_can.h"
 
-#define CAN_RAMINIT_START_MASK(i)	(0x001 << (i))
-#define CAN_RAMINIT_DONE_MASK(i)	(0x100 << (i))
-#define CAN_RAMINIT_ALL_MASK(i)		(0x101 << (i))
 #define DCAN_RAM_INIT_BIT		(1 << 3)
 static DEFINE_SPINLOCK(raminit_lock);
 /*
@@ -72,47 +71,61 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv,
 	writew(val, priv->base + 2 * priv->regs[index]);
 }
 
-static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask,
-				  u32 val)
+static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv,
+					 u32 mask, u32 val)
 {
 	int timeout = 0;
+	const struct c_can_raminit *raminit = &priv->raminit_sys;
+	u32 ctrl;
+
 	/* We look only at the bits of our instance. */
 	val &= mask;
-	while ((readl(priv->raminit_ctrlreg) & mask) != val) {
+	do {
 		udelay(1);
 		timeout++;
 
+		regmap_read(raminit->syscon, raminit->reg, &ctrl);
 		if (timeout == 1000) {
 			dev_err(&priv->dev->dev, "%s: time out\n", __func__);
 			break;
 		}
-	}
+	} while ((ctrl & mask) != val);
 }
 
-static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable)
+static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
 {
-	u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance);
+	u32 mask;
 	u32 ctrl;
+	const struct c_can_raminit *raminit = &priv->raminit_sys;
+	u8 start_bit, done_bit;
+
+	start_bit = raminit->start_bit;
+	done_bit = raminit->done_bit;
 
 	spin_lock(&raminit_lock);
 
-	ctrl = readl(priv->raminit_ctrlreg);
+	mask = 1 << start_bit | 1 << done_bit;
+	regmap_read(raminit->syscon, raminit->reg, &ctrl);
+
 	/* We clear the done and start bit first. The start bit is
 	 * looking at the 0 -> transition, but is not self clearing;
 	 * And we clear the init done bit as well.
+	 * NOTE: DONE must be written with 1 to clear it.
 	 */
-	ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance);
-	ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
-	writel(ctrl, priv->raminit_ctrlreg);
-	ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance);
-	c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+	ctrl &= ~(1 << start_bit);
+	ctrl |= 1 << done_bit;
+	regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+	ctrl &= ~(1 << done_bit);
+	c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
 
 	if (enable) {
 		/* Set start bit and wait for the done bit. */
-		ctrl |= CAN_RAMINIT_START_MASK(priv->instance);
-		writel(ctrl, priv->raminit_ctrlreg);
-		ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance);
-		c_can_hw_raminit_wait_ti(priv, mask, ctrl);
+		ctrl |= 1 << start_bit;
+		regmap_write(raminit->syscon, raminit->reg, ctrl);
+
+		ctrl |= 1 << done_bit;
+		c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
 	}
 	spin_unlock(&raminit_lock);
 }
@@ -206,10 +219,11 @@ static int c_can_plat_probe(struct platform_device *pdev)
 	struct net_device *dev;
 	struct c_can_priv *priv;
 	const struct of_device_id *match;
-	struct resource *mem, *res;
+	struct resource *mem;
 	int irq;
 	struct clk *clk;
 	const struct c_can_driver_data *drvdata;
+	struct device_node *np = pdev->dev.of_node;
 
 	match = of_match_device(c_can_of_table, &pdev->dev);
 	if (match) {
@@ -277,27 +291,50 @@ static int c_can_plat_probe(struct platform_device *pdev)
 		priv->read_reg32 = d_can_plat_read_reg32;
 		priv->write_reg32 = d_can_plat_write_reg32;
 
-		if (pdev->dev.of_node)
-			priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can");
-		else
-			priv->instance = pdev->id;
-
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		/* Not all D_CAN modules have a separate register for the D_CAN
-		 * RAM initialization. Use default RAM init bit in D_CAN module
-		 * if not specified in DT.
+		/* Check if we need custom RAMINIT via syscon. Mostly for TI
+		 * platforms. Only supported with DT boot.
 		 */
-		if (!res) {
+		if (np && of_property_read_bool(np, "syscon-raminit")) {
+			u32 id;
+			struct c_can_raminit *raminit = &priv->raminit_sys;
+
+			ret = -EINVAL;
+			raminit->syscon = syscon_regmap_lookup_by_phandle(np,
+									  "syscon-raminit");
+			if (IS_ERR(raminit->syscon)) {
+				/* can fail with -EPROBE_DEFER */
+				ret = PTR_ERR(raminit->syscon);
+				free_c_can_dev(dev);
+				return ret;
+			}
+
+			if (of_property_read_u32_index(np, "syscon-raminit", 1,
+						       &raminit->reg)) {
+				dev_err(&pdev->dev,
+					"couldn't get the RAMINIT reg. offset!\n");
+				goto exit_free_device;
+			}
+
+			if (of_property_read_u32_index(np, "syscon-raminit", 2,
+						       &id)) {
+				dev_err(&pdev->dev,
+					"couldn't get the CAN instance ID\n");
+				goto exit_free_device;
+			}
+
+			if (id >= drvdata->num_can) {
+				dev_err(&pdev->dev,
+					"Invalid CAN instance ID\n");
+				goto exit_free_device;
+			}
+
+			raminit->start_bit = drvdata->raminit_start_bits[id];
+			raminit->done_bit = drvdata->raminit_done_bits[id];
+
+			priv->raminit = c_can_hw_raminit_syscon;
+		} else {
 			priv->raminit = c_can_hw_raminit;
-			break;
 		}
-
-		priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start,
-						     resource_size(res));
-		if (!priv->raminit_ctrlreg || priv->instance < 0)
-			dev_info(&pdev->dev, "control memory is not used for raminit\n");
-		else
-			priv->raminit = c_can_hw_raminit_ti;
 		break;
 	default:
 		ret = -EINVAL;
-- 
1.8.3.2



^ permalink raw reply related

* [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-14 15:40 UTC (permalink / raw)
  To: wg, mkl
  Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
	sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros
In-Reply-To: <5464CCFF.5010004@ti.com>

DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.

To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/net/can/c_can/c_can.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8e78bb4..f94a9fa 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -35,6 +35,7 @@
 #include <linux/list.h>
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
 
 #include <linux/can.h>
 #include <linux/can/dev.h>
@@ -603,6 +604,8 @@ static int c_can_start(struct net_device *dev)
 
 	priv->can.state = CAN_STATE_ERROR_ACTIVE;
 
+	/* activate pins */
+	pinctrl_pm_select_default_state(dev->dev.parent);
 	return 0;
 }
 
@@ -611,6 +614,9 @@ static void c_can_stop(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	c_can_irq_control(priv, false);
+
+	/* deactivate pins */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
 	priv->can.state = CAN_STATE_STOPPED;
 }
 
@@ -1244,6 +1250,13 @@ int register_c_can_dev(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 	int err;
 
+	/* Deactivate pins to prevent DRA7 DCAN IP from being
+	 * stuck in transition when module is disabled.
+	 * Pins are activated in c_can_start() and deactivated
+	 * in c_can_stop()
+	 */
+	pinctrl_pm_select_sleep_state(dev->dev.parent);
+
 	c_can_pm_runtime_enable(priv);
 
 	dev->flags |= IFF_ECHO;	/* we support local echo */
-- 
1.8.3.2



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox