* Re: [PATCH v2 net-next 1/7] bpf: add 'flags' attribute to BPF_MAP_UPDATE_ELEM command
From: Alexei Starovoitov @ 2014-11-14 16:31 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: <1415981203.15154.45.camel@localhost>
On Fri, Nov 14, 2014 at 8:06 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Fr, 2014-11-14 at 07:33 -0800, Alexei Starovoitov wrote:
>> On Fri, Nov 14, 2014 at 4:11 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.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.
>
> I don't neither. ;)
>
> I just wanted to bring this up before user space api might get public
> and the additional argument might make problems.
addition of cmpxchg command won't be a problem obviously.
(just another 'new_value' field in existing struct inside bpf_attr union).
^ permalink raw reply
* Re: Understanding what's going on when using a Huawei E173 USB 3G web-stick (UMTS/HSPA)
From: Dan Williams @ 2014-11-14 16:18 UTC (permalink / raw)
To: sedat.dilek
Cc: Greg KH, David S. Miller, netdev@vger.kernel.org, linux-usb,
Aleksander Morgado
In-Reply-To: <CA+icZUX6caBKnjMv6V_26y_-VNfDqkeXFXKJhDdQpZy7W4oofw@mail.gmail.com>
On Fri, 2014-11-14 at 11:56 +0100, Sedat Dilek wrote:
> On Wed, Nov 12, 2014 at 2:21 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > On Tue, Nov 4, 2014 at 5:55 PM, Dan Williams <dcbw@redhat.com> 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"
Most WWAN devices actually require option, because most WWAN devices
have "serial" ports (even if they aren't used for PPP), and 'option' is
the driver that handles this. The 'option' name is historic, but the
driver should really be called something like 'wwan-serial-generic' or
something like that.
You're missing a few other "net" type drivers:
CONFIG_USB_NET_QMI_WWAN = Qualcomm QMI capable devices (net)
CONFIG_USB_HSO = "Option High-Speed" (net) devices
CONFIG_USB_NET_KALMIA = Samsung LTE dongle (net)
CONFIG_USB_SIERRA_NET = Sierra devices (net)
CONFIG_USB_NET_CDC_NCM = Ericsson F5321 (?) and some others (net)
CONFIG_USB_NET_HUAWEI_CDC_NCM = Huawei NCM variant (net)
CONFIG_USB_VL600 = LG VL600 / AD600 LTE device (net)
CONFIG_USB_NET_CDC_MBIM = MBIM (net) devices (popular currently)
and maybe even:
CONFIG_USB_CDC_PHONET = Nokia phones and USB sticks, not "net" but
proprietary protocol that handles both data/control channels
For the "serial" side:
CONFIG_USB_ACM = generic "serial" devices, many are *not* WWAN but many
WWAN devices use this class/driver
CONFIG_USB_SERIAL_QCAUX = Various Qualcomm-based devices' "auxiliary"
ports (DIAG, GPS, etc)
CONFIG_USB_SERIAL_QUALCOMM = Firmware loading and "auxiliary" ports on
various Qualcomm Gobi devices
CONFIG_USB_SERIAL_SIERRAWIRELESS = Older Sierra device serial ports for
PPP/control and "auxiliary" functions
WWAN devices basically mix & match these drivers depending on what
interfaces the firmware exposes.
For example, some Sierra devices may require both
CONFIG_USB_SERIAL_SIERRAWIRELESS and CONFIG_USB_SIERRA_NET.
Older Sierra devices may use only CONFIG_USB_SERIAL_SIERRAWIRELESS and
do not provide a 'net' port at all, but use only PPP.
Sierra devices based on Icera chips may use CONFIG_USB_ACM and either
CONFIG_USB_SIERRA_NET or CONFIG_USB_NET_CDCETHER.
Some Huawei devices may use CONFIG_USB_NET_CDCETHER and either
CONFIG_USB_SERIAL_OPTION or CONFIG_USB_ACM.
Other Huawei devices may use CONFIG_USB_NET_QMI_WWAN and
CONFIG_USB_SERIAL_OPTION.
Even other Huawei devices may be Qualcomm Gobi type and use
CONFIG_USB_NET_QMI_WWAN and CONFIG_USB_SERIAL_QUALCOMM.
So you see it really depends on exactly how the firmware is implemented.
But in general, devices still fall into the categories I originally
listed, and the drivers fall into specific categories too ("net",
"serial", "proprietary"), and devices mix & match drivers to achieve the
end result.
Dan
> ### 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 netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Rick Jones @ 2014-11-14 16:16 UTC (permalink / raw)
To: Eric Dumazet, Rick Jones; +Cc: netdev, davem
In-Reply-To: <1415931471.17262.27.camel@edumazet-glaptop2.roam.corp.google.com>
>
> This looks quite complicated to me.
>
> Why are you adding kfree_skb() everywhere instead of :
>
> bool to_consume = icmp_pointers[icmph->type].handler(skb);
> if (ro_consume)
> consume_skb(skb);
> else
> kfree_skb(skb);
I thought the point of the drop profiling was to show where the drops
were happening. Leaving the kfree_skb() up in icmp_rcv() does not
improve showing where the drops happened. That is why I've pushed it
down into the routines called by icmp_rcv().
rick
^ 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 16:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Eric Dumazet, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUux2vuN3iNaAsxybyFwzfMRVe1+BOW-JvnvYYh+=-vpELw@mail.gmail.com>
On Fr, 2014-11-14 at 07:33 -0800, Alexei Starovoitov wrote:
> On Fri, Nov 14, 2014 at 4:11 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.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.
I don't neither. ;)
I just wanted to bring this up before user space api might get public
and the additional argument might make problems.
Bye,
Hannes
^ permalink raw reply
* Re: [PATCH v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Roger Quadros @ 2014-11-14 16:04 UTC (permalink / raw)
To: Marc Kleine-Budde, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <54662493.5040002@pengutronix.de>
On 11/14/2014 05:49 PM, Marc Kleine-Budde wrote:
> On 11/14/2014 04:40 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>
>
> Wow! Some interations later this patch got quite nice and shiny :)
> Applied all to can-next/master.
Thanks Marc :)
cheers,
-roger
^ permalink raw reply
* [PATCH] e100: fix typo in MDI/MDI-X eeprom check in e100_phy_init
From: John W. Linville @ 2014-11-14 15:59 UTC (permalink / raw)
To: netdev
Cc: Dave Miller, Jeff Kirsher, Auke Kok, Malli Chilakala,
John W. Linville
Although it doesn't explicitly say so, commit 60ffa478759f39a2 ("e100:
Fix MDIO/MDIO-X") appears to be intended to revert the earlier commit
648951451e6d2d53 ("e100: fixed e100 MDI/MDI-X issues"). However,
careful examination reveals that the attempted revert actually
_inverted_ the test for eeprom_mdix_enabled. That is bound to program
a few PHYs incorrectly...
https://bugzilla.redhat.com/show_bug.cgi?id=1156417
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Auke Kok <auke-jan.h.kok@intel.com>
Cc: Malli Chilakala <mallikarjuna.chilakala@intel.com>
---
Wow, an 8 year old bug in e100 -- woohoo!! :-)
This was causing some serious flakiness for a large cash register
deployment in Europe. Testing with this one-line (really,
one-character) patch seems to have resolved the issue.
drivers/net/ethernet/intel/e100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 781065eb5431..e9c3a87e5b11 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1543,7 +1543,7 @@ static int e100_phy_init(struct nic *nic)
mdio_write(netdev, nic->mii.phy_id, MII_BMCR, bmcr);
} else if ((nic->mac >= mac_82550_D102) || ((nic->flags & ich) &&
(mdio_read(netdev, nic->mii.phy_id, MII_TPISTATUS) & 0x8000) &&
- !(nic->eeprom[eeprom_cnfg_mdix] & eeprom_mdix_enabled))) {
+ (nic->eeprom[eeprom_cnfg_mdix] & eeprom_mdix_enabled))) {
/* enable/disable MDI/MDI-X auto-switching. */
mdio_write(netdev, nic->mii.phy_id, MII_NCONFIG,
nic->mii.force_media ? 0 : NCONFIG_AUTO_SWITCH);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 1/3] arch: Introduce load_acquire() and store_release()
From: Alexander Duyck @ 2014-11-14 16:00 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arch@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mikey@neuling.org,
tony.luck@intel.com, mathieu.desnoyers@polymtl.ca,
donald.c.skidmore@intel.com, peterz@infradead.org,
benh@kernel.crashing.org, heiko.carstens@de.ibm.com,
oleg@redhat.com, davem@davemloft.net, michael@ellerman.id.au,
matthew.vick@intel.com, nic_swsd@realtek.com,
geert@linux-m68k.org, jeffrey.t.kirsher@intel.com
In-Reply-To: <20141114101902.GA27963@arm.com>
On 11/14/2014 02:19 AM, Will Deacon wrote:
> Hi Alex,
>
> On Thu, Nov 13, 2014 at 07:27:23PM +0000, Alexander Duyck wrote:
>> It is common for device drivers to make use of acquire/release semantics
>> when dealing with descriptors stored in device memory. On reviewing the
>> documentation and code for smp_load_acquire() and smp_store_release() as
>> well as reviewing an IBM website that goes over the use of PowerPC barriers
>> at http://www.ibm.com/developerworks/systems/articles/powerpc.html it
>> occurred to me that the same code could likely be applied to device drivers.
>>
>> As a result this patch introduces load_acquire() and store_release(). The
>> load_acquire() function can be used in the place of situations where a test
>> for ownership must be followed by a memory barrier. The below example is
>> from ixgbe:
>>
>> if (!rx_desc->wb.upper.status_error)
>> break;
>>
>> /* This memory barrier is needed to keep us from reading
>> * any other fields out of the rx_desc until we know the
>> * descriptor has been written back
>> */
>> rmb();
>>
>> With load_acquire() this can be changed to:
>>
>> if (!load_acquire(&rx_desc->wb.upper.status_error))
>> break;
> I still don't think this is a good idea for the specific use-case you're
> highlighting.
>
> On ARM, an mb() can be *significantly* more expensive than an rmb() (since
> we may have to drain store buffers on an outer L2 cache) and on arm64 it's
> not at all clear that an LDAR is more efficient than an LDR; DMB LD
> sequence. I can certainly imagine implementations where the latter would
> be preferred.
Yeah, I am pretty sure I overdid it in using a mb() for arm. I think
what I should probably be using is something like dmb(ish) which is used
for smp_mb() instead. The general idea is to enforce memory-memory
accesses. The memory-mmio accesses still should be using a full
rmb()/wmb() barrier.
The alternative I am mulling over is creating something like a
lightweight set of memory barriers named lw_mb(), lw_rmb(), lw_wmb(),
that could be used instead. The general idea is that on many
architectures a full mb/rmb/wmb is far too much for just guaranteeing
ordering for system memory only writes or reads. I'm thinking I could
probably use the smp_ varieties as a template for them since I'm
thinking that in most cases this should be correct.
Also, just to be clear I am not advocating replacing the wmb() in most
I/O setups where we have to sync the system memory before doing the MMIO
write. This is for the case where the device descriptor ring has some
bit indicating ownership by either the device or the CPU. So for
example on the r8169 they have to do a wmb() before writing the DescOwn
bit in the first descriptor of a given set of Tx descriptors to
guarantee the rest are written, then they set the DescOwn bit, then they
call wmb() again to flush that last bit before notifying the device it
can start fetching the descriptors. My goal is to deal with that first
wmb() and leave the second as it since it is correct.
> So, whilst I'm perfectly fine to go along with mandatory acquire/release
> macros (we should probably add a check to barf on __iomem pointers), I
> don't agree with using them in preference to finer-grained read/write
> barriers. Doing so will have a real impact on I/O performance.
Couldn't that type of check be added to compiletime_assert_atomic_type?
That seems like that would be the best place for something like that.
> Finally, do you know of any architectures where load_acquire/store_release
> aren't implemented the same way as the smp_* variants on SMP kernels?
>
> Will
I should probably go back through and sort out the cases where mb() and
smp_mb() are not the same thing. I think I probably went with too harsh
of a barrier in probably a couple of other cases.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v4 7/8] net: can: c_can: Add support for TI DRA7 DCAN
From: Marc Kleine-Budde @ 2014-11-14 15:56 UTC (permalink / raw)
To: Roger Quadros, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <1415371762-29885-8-git-send-email-rogerq@ti.com>
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
On 11/07/2014 03:49 PM, Roger Quadros wrote:
> DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
> register data for both.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> Documentation/devicetree/bindings/net/can/c_can.txt | 1 +
> drivers/net/can/c_can/c_can_platform.c | 11 +++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
> index a3ca3ee..f682fdb 100644
> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
> @@ -4,6 +4,7 @@ Bosch C_CAN/D_CAN controller Device Tree Bindings
> Required properties:
> - compatible : Should be "bosch,c_can" for C_CAN controllers and
> "bosch,d_can" for D_CAN controllers.
> + Can be "ti,dra7-d_can".
> - reg : physical base address and size of the C_CAN/D_CAN
> registers map
> - interrupts : property with a value describing the interrupt
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 71b9063..7a81db4 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -195,6 +195,16 @@ static struct c_can_driver_data d_can_drvdata = {
> .id = BOSCH_D_CAN,
> };
>
> +static u8 dra7_raminit_start_bits[] = {3, 5};
> +static u8 dra7_raminit_done_bits[] = {1, 2};
> +static struct c_can_driver_data dra7_dcan_drvdata = {
> + .id = BOSCH_D_CAN,
> + .num_can = 2,
^
Replaced by ARRAY_SIZE(dra7_raminit_start_bits)
Same for the am3352_dcan_drvdata in the next patch.
> + .raminit_start_bits = dra7_raminit_start_bits,
> + .raminit_done_bits = dra7_raminit_done_bits,
> + .raminit_pulse = true,
> +};
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 v7 6/8] net: can: c_can: Disable pins when CAN interface is down
From: Marc Kleine-Budde @ 2014-11-14 15:49 UTC (permalink / raw)
To: Roger Quadros, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <5466225D.2070202@ti.com>
[-- Attachment #1: Type: text/plain, Size: 866 bytes --]
On 11/14/2014 04:40 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>
Wow! Some interations later this patch got quite nice and shiny :)
Applied all to can-next/master.
Thanks,
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:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, ogerlitz, pshelar, jesse, jay.vosburgh, discuss
In-Reply-To: <1415979181.17262.45.camel@edumazet-glaptop2.roam.corp.google.com>
On Fr, 2014-11-14 at 07:33 -0800, Eric Dumazet wrote:
> 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.
__intel_crc4_2_hash(2) does already make use of crc32 instruction. I'll
have a closer look at what gcc generates.
> 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.
Ok, understood, so we only clobber the registers needed in the
crc32_hash implementation and only if we branch to jhash we save all the
other ones in a trampoline directly before jhash.
> 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.
Yes, makes sense.
I would still like to see the current proposed fix getting applied and
we can do this on-top. The inline call after this patch reassembles a
direct function call, so besides the long list of clobbers, it should
still be pretty fast.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH] rtlwifi: Add more checks for get_btc_status callback
From: Mike Galbraith @ 2014-11-14 15:41 UTC (permalink / raw)
To: Murilo Opsfelder Araújo
Cc: Larry Finger, linux-kernel, linux-wireless, netdev, Chaoming Li,
John W. Linville, Thadeu Cascardo, 谭杭波
In-Reply-To: <CA+LKeY6xca=f-qFJoVFaqywRpfHPmMAWDrMmbkUiDf9atA_jVQ@mail.gmail.com>
On Wed, 2014-11-12 at 21:03 -0200, Murilo Opsfelder Araújo wrote:
> Hello, Larry.
>
> I'd like to let you know that next-20141112 brought my rtl8192se
> device back to life. I didn't need to build rtlwifi_new to be able to
> connect to my wifi network. It worked just fine.
Yeah, unsurprisingly, merging those fixed up my lappy as well.
-Mike
^ permalink raw reply
* [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
* [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
* 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
* 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 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 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
* [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 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
* 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 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: 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 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
* [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
* [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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox