From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Stefano Babic <sbabic@denx.de>, linux-can@vger.kernel.org
Cc: Marc Kleine-Budde <mkl@pengutronix.de>,
Wolfgang Grandegger <wg@grandegger.com>
Subject: Re: [PATCH v3 2/2] CAN: CAN driver to support multiple CAN bus on SPI interface
Date: Wed, 02 Apr 2014 19:21:12 +0200 [thread overview]
Message-ID: <533C4708.8030600@hartkopp.net> (raw)
In-Reply-To: <1396423121-28949-3-git-send-email-sbabic@denx.de>
Hello Stefano,
On 02.04.2014 09:18, Stefano Babic wrote:
> +
> +/* CFG message */
> +struct msg_cfg_data {
> + u8 channel;
> + u8 enabled;
> + struct can_bittiming bt;
> +} __packed;
> +static int spi_can_probe(struct spi_device *spi)
> +{
(..)
> +
> + /* Set CAN specific parameters */
> + priv->can.clock.freq = SLAVE_CLK_FREQ;
> + priv->can.bittiming_const = NULL;
> + priv->can.do_set_mode = spi_can_set_mode;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_3_SAMPLES;
This looks wrong to me.
In the one case (ctrlmode_supported) you aim to know about the SPI CAN
controllers e.g. if they support LISTENONLY.
In the other case (bittiming_const) you don't know anything about the
controller??
How do you set the bitrate without defining bittiming_const??
And how does
ip -det link show can0
look like for your device then?
Setting the bitrate with the ip tools requires bittiming_const to be set:
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/drivers/net/can/dev.c#n252
IMO both ctrlmode_supported and bittiming_const need to be retrieved from
each remote SPI attached CAN controller at startup initialization time.
So that the configuration can be done with the standard tools.
Regards,
Oliver
next prev parent reply other threads:[~2014-04-02 17:21 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-02 7:18 [PATCH v3 0/2] Adding support for CAN busses via SPI interface Stefano Babic
2014-04-02 7:18 ` [PATCH v3 1/2] Add documentation for SPI to CAN driver Stefano Babic
2014-04-02 7:18 ` [PATCH v3 2/2] CAN: CAN driver to support multiple CAN bus on SPI interface Stefano Babic
2014-04-02 17:21 ` Oliver Hartkopp [this message]
2014-04-03 8:57 ` Stefano Babic
2014-04-03 12:02 ` Oliver Hartkopp
2014-04-03 12:14 ` 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=533C4708.8030600@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=linux-can@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=sbabic@denx.de \
--cc=wg@grandegger.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).