All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfgang Grandegger <wg@grandegger.com>
To: Oliver Hartkopp <socketcan@hartkopp.net>,
	Stefano Babic <sbabic@denx.de>,
	linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Subject: Re: [PATCH v2 3/3] CAN: CAN driver to support multiple CAN bus on SPI interface
Date: Sun, 06 Apr 2014 21:22:14 +0200	[thread overview]
Message-ID: <5341A966.5080202@grandegger.com> (raw)
In-Reply-To: <5341A497.9090409@hartkopp.net>

On 04/06/2014 09:01 PM, Oliver Hartkopp wrote:
> Hello Wolfgang,
> 
> On 06.04.2014 17:20, Wolfgang Grandegger wrote:
>> Hi Stefan,
>>
>> sorry for jumping in late... 
> 
> indeed.
> 
> There were some hints from me which already led to a v3 which would be better
> to review ;-)

Oops, I picked the wrong one.

>> This driver is rather special in various
>> respects. As I see it, it does not support:
>>
>>   - loopback (echo_skbs)
>>   - error state handling
>>   - any error reporting (CAN error messages)
>>   - any error counting (net and CAN stats)
>>   - recovery from bus-off
>>
>> Any chance to improve on that? At least some kind of error reporting
>> would be nice otherwise the app does not realize any problems. Or how
>> does your app handle error cases.
> 
> You are right on asking this.
> 
> On the other had I suggested to name it "spi_can" in all places to make it a
> generic driver to attach micro controllers which have CAN and SPI in a common way.

But it depends on the firmware running on the micro controller. I still
think that "spi_can" will confuse people. Anyway, just a name.

> We already remove the 'specific' naming for the iMX35 and HCS12 to make it a
> generic driver and not a specifc driver for this setup.
> 
> Please check the v3 - any also my comments about the bitrate setting
> configuration to be handled by SocketCAN.
> 
> Your remarks from above just go into the same direction.

Yes, some issues seem to have vanished.

Wolfgang.


  reply	other threads:[~2014-04-06 19:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-25 14:30 [PATCH v2 0/3] Adding support for CAN busses via SPI interface Stefano Babic
2014-03-25 14:30 ` [PATCH v2 1/3] Add documentation for SPI to CAN driver Stefano Babic
2014-03-25 18:14   ` Oliver Hartkopp
2014-03-26  8:07     ` Stefano Babic
2014-03-25 14:30 ` [PATCH v2 2/3] CAN: moved SPI drivers into a separate directory Stefano Babic
2014-03-25 18:56   ` Oliver Hartkopp
2014-04-01 21:08   ` Marc Kleine-Budde
2014-03-25 14:30 ` [PATCH v2 3/3] CAN: CAN driver to support multiple CAN bus on SPI interface Stefano Babic
2014-03-25 18:54   ` Oliver Hartkopp
2014-03-26  9:01     ` Stefano Babic
2014-04-06 15:20   ` Wolfgang Grandegger
2014-04-06 19:01     ` Oliver Hartkopp
2014-04-06 19:22       ` Wolfgang Grandegger [this message]
2014-04-06 19:42         ` Oliver Hartkopp
2014-04-07  8:29     ` Stefano Babic
2014-04-07  9:34       ` Wolfgang Grandegger
2014-04-07 10:24         ` Stefano Babic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5341A966.5080202@grandegger.com \
    --to=wg@grandegger.com \
    --cc=linux-can@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=sbabic@denx.de \
    --cc=socketcan@hartkopp.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.