As Dan Carpenter reported in http://marc.info/?l=linux-can&m=144793696016187
the assignment of the error location in CAN error messages had some bit wise
overlaps. Indeed the value to be assigned in data[3] is no bitfield but defines
a single value which points to a location inside the CAN frame on the wire.

This patch fixes the assignments for the error locations in error messages.
Reported-by: NDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NOliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

The previous change 3973c526 (net: can: c_can: Disable pins when CAN
interface is down) causes a slight glitch on the pinctrl settings when used.
Since commit ab78029e (drivers/pinctrl: grab default handles from device core),
the device core will automatically set the default pins. This causes the pins
to be momentarily set to the default and then to the sleep state in
register_c_can_dev(). By adding an optional "enable" state, boards can set the
default pin state to be disabled and avoid the glitch when the switch from
default to sleep first occurs. If the "enable" state is not available
c_can_pinctrl_select_state() falls back to using the "default" pinctrl state.

[Roger Q] - Forward port to v4.2 and use pinctrl_get_select().
Signed-off-by: NJ.D. Schroeder <jay.schroeder@garmin.com>
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Reviewed-by: NGrygorii Strashko <grygorii.strashko@ti.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Put controller into init mode in network stop to end pending transmissions. The
issue is observed in cases when transmitted frame is not acked.
Signed-off-by: NViktor Babrian <babrian.viktor@renyi.mta.hu>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

In order to be able to move the stats increment from can_bus_off() into
can_change_state(), the increment had to be moved back into code that was using
can_bus_off() but not can_change_state().

As a side-effect, this patch fixes the following bugs:
 * Redundant call to can_bus_off() in c_can.
 * Bus-off counted twice in xilinx_can.
Signed-off-by: NAndri Yngvason <andri.yngvason@marel.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

use of regmap_read() and regmap_write() in c_can_hw_raminit_syscon()
is not safe as the RAMINIT register can be shared between different drivers
at least for TI SoCs.

To make the modification atomic we switch to using regmap_update_bits().

regmap_update_bits() skips writing to the register if it's read content is the
same as what is going to be written. This causes an issue for us when we
need to clear the DONE bit with the initial condition START:0, DONE:1 as
DONE bit must be written with 1 to clear it.

So we defer the clearing of DONE bit to later when we set the START bit.
There we are sure that START bit is changed from 0 to 1 so the write of
1 to already set DONE bit will happen.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

AM4372 SoC has 2 DCAN modules. Add compatible id and
raminit driver data for it. The driver data is same as AM3352
but this gives us flexibility to add AM4372 specific quirks
if required later.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

AM3352 SoC has 2 DCAN modules. Add compatible id and
raminit driver data for am3352 DCAN.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Acked-by: NWolfram Sang <wsa@the-dreams.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

DRA7 SoC has 2 CAN IPs. Provide compatible IDs and RAMINIT
register data for both.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

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: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Some SoCs e.g. (TI DRA7xx) need a START pulse to start the
RAMINIT sequence i.e. START bit must be set and cleared before
checking for the DONE bit status.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

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: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Some platforms (e.g. TI) need special RAMINIT register handling.
Provide a way to store RAMINIT register description in driver data.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

We want to have more data than just can_dev_id to be present
in the driver data e.g. TI platforms need RAMINIT register
description. Introduce the c_can_driver_data structure and move
the can_dev_id into it.

Tidy up the way it is used on probe().
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

TI's RAMINIT DONE mechanism is buggy on AM43xx SoC and may not always
be set after the START bit is set. Although it seems to work fine even
in that case. So add a timeout mechanism to c_can_hw_raminit_wait_ti().
Don't bail out in that failure case but just print an error message.
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

A platform_driver does not need to set an owner, it will be populated by the
driver core.
Signed-off-by: NWolfram Sang <wsa@the-dreams.de>

Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). They
seem to have been swapped in the usage instances.
Reported-by: NJay Schroeder <jay.schroeder@garmin.com>
Signed-off-by: NRoger Quadros <rogerq@ti.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

devm_ioremap() returns NULL on error, not an ERR_PTR().

Fixes: 33cf7565 ('can: c_can_platform: Fix raminit, use devm_ioremap() instead of devm_ioremap_resource()')
Signed-off-by: NDan Carpenter <dan.carpenter@oracle.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v3.11
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

No need to manually copy debug settings into subdir Makefiles. kbuild
has a mechanism for inheriting, so let's use it.
Signed-off-by: NWolfram Sang <wsa@the-dreams.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

We should prefer `struct pci_device_id` over `DEFINE_PCI_DEVICE_TABLE` to
meet kernel coding style guidelines.  This issue was reported by checkpatch.

A simplified version of the semantic patch that makes this change is as
follows (http://coccinelle.lip6.fr/):

// <smpl>

@@
identifier i;
declarer name DEFINE_PCI_DEVICE_TABLE;
initializer z;
@@

- DEFINE_PCI_DEVICE_TABLE(i)
+ const struct pci_device_id i[]
= z;

// </smpl>

[bhelgaas: add semantic patch]
Signed-off-by: NBenoit Taine <benoit.taine@lip6.fr>
Signed-off-by: NBjorn Helgaas <bhelgaas@google.com>

The raminit register is shared register for both can0 and can1. Since commit:

    32766fff net: can: Convert to use devm_ioremap_resource

devm_ioremap_resource() is used to map raminit register. When using both
interfaces the mapping for the can1 interface fails, leading to a non
functional can interface.
Signed-off-by: NGeorge Cherian <george.cherian@ti.com>
Signed-off-by: NMugunthan V N <mugunthanvnm@ti.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v3.11
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

This patch uses devm_* APIs as they are device managed and make code simpler.
Signed-off-by: NLad, Prabhakar <prabhakar.csengg@gmail.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Non-TI chips (including socfpga) needs different raminit sequence. Implement
it.
Tested-by: NThor Thayer <tthayer@altera.com>
Signed-off-by: NThor Thayer <tthayer@altera.com>
Signed-off-by: NPavel Machek <pavel@denx.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Add helpers for 32-bit accesses and replace open-coded 32-bit access
with calls to helpers. Minimum changes are done to the pci case, as I
don't have access to that hardware.
Tested-by: NThor Thayer <tthayer@altera.com>
Signed-off-by: NThor Thayer <tthayer@altera.com>
Signed-off-by: NPavel Machek <pavel@denx.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

This patch makes the {read,write}_reg functions const, this is a preparation to
make use of {read,write}_reg in the hwinit callback.
Signed-off-by: NThor Thayer <tthayer@altera.com>
Signed-off-by: NPavel Machek <pavel@denx.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

In 2b9aecdc ("can: c_can: Disable rx split as workaround") a new Kconfig
option was introduced as a workaround. The tests performed by Alexander Stein
confirmed this option to be obsolete with all the other cleanups and fixes
that had been discussed that time:
http://marc.info/?l=linux-can&m=139746476821294&w=2

Both (author and tester) agreed to remove this Kconfig option again:
http://marc.info/?l=linux-can&m=139883820714229&w=2

As some more cleanups took place since then a simple revert is not possible.
This patch removes the entire option as it would behave when disabled.
Further beautification’s can be done later.
Signed-off-by: NOliver Hartkopp <socketcan@hartkopp.net>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Signed-off-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Acked-by: NWolfgang Grandegger <wg@grandegger.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Coverity complains that c_can_pci_probe() calls pci_enable_msi() without
checking the result:

CID 712278 (#1 of 1): Unchecked return value (CHECKED_RETURN) 3. check_return:
Calling pci_enable_msi_block without checking return value (as is done
elsewhere 88 out of 105 times).
 88        pci_enable_msi(pdev);

This is CID 712278.
Signed-off-by: NWolfgang Grandegger <wg@grandegger.com>
Reported-by: NBjorn Helgaas <bhelgaas@google.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Commit 6439fbce (can: c_can: fix error checking of priv->instance in
probe()) found the warning but applied a suboptimal solution. Since, both
pdev->id and of_alias_get_id() return integers, it makes sense to convert the
variable to an integer and avoid the cast.
Signed-off-by: NWolfram Sang <wsa@sang-engineering.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

It's suffcient to kill the TXIE bit in the message control register
even if the documentation of C and D CAN says that it's not allowed to
do that while MSGVAL is set. Reality tells a different story and this
change gives us another 2% of CPU back for not waiting on I/O.
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Mark suggested to use one IF for the softirq and the other for the
xmit function to avoid the xmit lock.

That requires to write the frame into the interface first, then handle
the echo skb and store the dlc before committing the TX request to the
message ram.

We use an atomic to handle the active buffers instead of reading the
MSGVAL register as thats way faster especially on PCH/x86.
Suggested-by: NMark <mark5@del-llc.com>
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Instead of obfuscating the code by artificial 16 bit splits use the
proper 32 bit assignments and split the result when writing to the
interface.
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Remove the MASK from the TX transfer side.

Make the code readable and get rid of the annoying IFX_WRITE_XXX_16BIT
macros which are just obfuscating the code.
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Sigh!
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

Alexander reported that the new optimized handling of the RX fifo
causes random packet loss on Intel PCH C_CAN hardware.

After a few fruitless debugging sessions I got hold of a PCH (eg20t)
afflicted system. That machine does not have the CAN interface wired
up, but it was possible to reproduce the issue with the HW loopback
mode.

As Alexander observed correctly, clearing the NewDat flag along with
reading out the message buffer causes that issue on C_CAN, while D_CAN
handles that correctly.

Instead of restoring the original message buffer handling horror the
following workaround solves the issue:

    transfer buffer to IF without clearing the NewDat
    handle the message
    clear NewDat bit

That's similar to the original code but conditional for C_CAN.

I really wonder why all user manuals (C_CAN, Intel PCH and some more)
recommend to clear the NewDat bit right away. The knows it all Oracle
operated by Gurgle does not unearth any useful information either. I
simply cannot believe that we are the first to uncover that HW issue.
Reported-and-tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

The RX buffer split causes packet loss in the hardware:

What happens is:

RX Packet 1 --> message buffer 1 (newdat bit is not cleared)
RX Packet 2 --> message buffer 2 (newdat bit is not cleared)
RX Packet 3 --> message buffer 3 (newdat bit is not cleared)
RX Packet 4 --> message buffer 4 (newdat bit is not cleared)
RX Packet 5 --> message buffer 5 (newdat bit is not cleared)
RX Packet 6 --> message buffer 6 (newdat bit is not cleared)
RX Packet 7 --> message buffer 7 (newdat bit is not cleared)
RX Packet 8 --> message buffer 8 (newdat bit is not cleared)

Clear newdat bit in message buffer 1
Clear newdat bit in message buffer 2
Clear newdat bit in message buffer 3
Clear newdat bit in message buffer 4
Clear newdat bit in message buffer 5
Clear newdat bit in message buffer 6
Clear newdat bit in message buffer 7
Clear newdat bit in message buffer 8

Now if during that clearing of newdat bits, a new message comes in,
the HW gets confused and drops it.

It does not matter how many of them you clear. I put a delay between
clear of buffer 1 and buffer 2 which was long enough that the message
should have been queued either in buffer 1 or buffer 9. But it did not
show up anywhere. The next message ended up in buffer 1. So the
hardware lost a packet of course without telling it via one of the
error handlers.

That does not happen on all clear newdat bit events. I see one of 10k
packets dropped in the scenario which allows us to reproduce. But the
trace looks always the same.

Not splitting the RX Buffer avoids the packet loss but can cause
reordering. It's hard to trigger, but it CAN happen.

With that mode we use the HW as it was probably designed for. We read
from the buffer 1 upwards and clear the buffer as we get the
message. That's how all microcontrollers use it. So I assume that the
way we handle the buffers was never really tested. According to the
public documentation it should just work :)

Let the user decide which evil is the lesser one.

[ Oliver Hartkopp: Provided a sane config option and help text and
  made me switch to favour potential and unlikely reordering over
  packet loss ]
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>

The driver handles pointlessly TWO interrupts per packet. The reason
is that it enables the status interrupt which fires for each rx and tx
packet and it enables the per message object interrupts as well.

The status interrupt merily acks or in case of D_CAN ignores the TX/RX
state and then the message object interrupt fires.

The message objects interrupts are only useful if all message objects
have hardware filters activated.

But we don't have that and its not simple to implement in that driver
without rewriting it completely.

So we can ditch the message object interrupts and handle the RX/TX
right away from the status interrupt. Instead of TWO we handle ONE.

Note: We must keep the TXIE/RXIE bits in the message buffers because
the status interrupt alone is not reliable enough in corner cases.

If we ever have the need for HW filtering, then this code needs a
complete overhaul and we can think about it then. For now we prefer a
lower interrupt load.
Signed-off-by: NThomas Gleixner <tglx@linutronix.de>
Tested-by: NAlexander Stein <alexander.stein@systec-electronic.com>
Signed-off-by: NMarc Kleine-Budde <mkl@pengutronix.de>