netdev - Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4C7F450C.4010002@grandegger.com>
Date:	Thu, 02 Sep 2010 08:32:44 +0200
From:	Wolfgang Grandegger <wg@...ndegger.com>
To:	Masayuki Ohtake <masa-korg@....okisemi.com>
CC:	Andrew Chih Howe <andrew.chih.howe.khor@...el.com>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	ML netdev <netdev@...r.kernel.org>, gregkh@...e.de,
	"Wang, Yong Y" <yong.y.wang@...el.com>,
	ML linux-kernel <linux-kernel@...r.kernel.org>,
	socketcan-core@...ts.berlios.de,
	Morinaga <morinaga526@....okisemi.com>, arjan@...ux.intel.com,
	"David S. Miller" <davem@...emloft.net>,
	Christian Pellegrin <chripell@...e.org>, Qi <qi.wang@...el.com>
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35

On 09/02/2010 05:19 AM, Masayuki Ohtake wrote:
> ----- Original Message ----- 
> From: "Wolfgang Grandegger" <wg@...ndegger.com>
> To: "Masayuki Ohtake" <masa-korg@....okisemi.com>
> Cc: "Andrew Chih Howe" <andrew.chih.howe.khor@...el.com>; "Qi" <qi.wang@...el.com>; "ML netdev"
> <netdev@...r.kernel.org>; <gregkh@...e.de>; "ML linux-kernel" <linux-kernel@...r.kernel.org>; "Wang, Yong Y"
> <yong.y.wang@...el.com>; <socketcan-core@...ts.berlios.de>; <arjan@...ux.intel.com>; "David S. Miller"
> <davem@...emloft.net>; "Christian Pellegrin" <chripell@...e.org>; "Samuel Ortiz" <sameo@...ux.intel.com>
> Sent: Thursday, September 02, 2010 3:51 AM
> Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35
> 
>> ...
>>>>>>> - The values for the hw-specific bit-timing registers should be derived
>>>>>>>   from the calculated values in "priv->can.bittiming":
>>>>>>>
>>>>>>>   http://lxr.linux.no/#linux+v2.6.35/include/linux/can/netlink.h#L17
>>>>>>>
>>>
>>> I show current pch_can code below.
>>>
>>> +static int pch_set_bittiming(struct net_device *ndev)
>>> +{
>>> + struct pch_can_priv *priv = netdev_priv(ndev);
>>> + struct pch_can_os *dev_can_os = priv->pch_can_os_p;
>>> + const struct can_bittiming *bt = &priv->can.bittiming;
>>>
>>> Is the above TRUE, isn't it ?
>>
>> The code fragment looks good. In that function you should then *derive*
>> the values of the bit-timing registers from the data fields of "bt". For
>> the SJA1000, you find the code here:
>>
>> http://lxr.linux.no/#linux+v2.6.35/drivers/net/can/sja1000/sja1000.c#L202
>>
> 
> I can't understand the your saying.
> I think our can driver is implemented like your saying.
> 
> In function "pch_set_bittiming", get the value of the bit-timing registers from
> the data fields of "bt" at "pch_can_set_baud_custom" or "pch_can_set_baud_simple".

Please *remove* "pch_can_set_baud_custom" or "pch_can_set_baud_simple"
and use the fields of "const struct can_bittiming *bt" *directly*:

  /* Getting the appropriate register value. */
  reg_val = (((bt->brp & MSK_BITT_BRP) << BIT_BITT_BRP) |
	    ((bt->prop_seg + bt->phase_seg1 - 1) << BIT_BITT_TSEG1) |
            ...

> Could you indicate in more detail ?

Please have a closer look to the link mentioned above.

>>>>>>> - The driver should handle state changes and communicate them to the
>>>>>>>   user space via error messages, if possible.
>>>>>>>
>>> What's "state chage" mean ?
>>
>> Googling for "can bis states" returned:
>>
>>
> http://www.softing.com/home/en/industrial-automation/products/can-bus/more-can-bus/error-handling/error-states.php?navanchor=3010510
>>
>> The CAN controller usually triggers an interrupt when the state changes,
>> which allows the driver to track the CAN state and deliver that
>> information to the user space.
> 
> I could understand your saying.
> In our current code, our driver can detect state change, but doesn't notify to
> can-core module or kennel protocol stack.
> We will modify our driver to notify to these.

The code is your friend. Please have a more detailed look to the SJA1000
driver, e.g. search it for "state".

Wolfgang.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html