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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 14 Sep 2010 09:46:36 +0900
From:	"Masayuki Ohtake" <masa-korg@....okisemi.com>
To:	"Marc Kleine-Budde" <mkl@...gutronix.de>
Cc:	"Wolfgang Grandegger" <wg@...ndegger.com>, <arjan@...ux.intel.com>,
	<meego-dev@...go.com>, "Morinaga" <morinaga526@....okisemi.com>,
	<socketcan-core@...ts.berlios.de>, <yong.y.wang@...el.com>,
	<gregkh@...e.de>, <netdev@...r.kernel.org>, <qi.wang@...el.com>,
	<andrew.chih.howe.khor@...el.com>
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35

Hi Marc,

>>> - implement NAPI
>> Since Topcliff CAN HW register has only single rx buffer,
>> I think NAPI is unnecessary.

> Doesn't matter. Please try to implement it.

Our CAN driver must pull received data from CAN-HW rx buffer as fast as it can
so that the received data is not over-written by next received data.
In case of implemented with NAPI,
since NAPI has time-lagging after receiving first packet,
probability of over-written(discarded) buffer is to be high.
Thus, for our CAN HW, we should NOT implement with NAPI but normal "netif_rx".

Thanks, Ohtake(OKISemi)

----- Original Message ----- 
From: "Marc Kleine-Budde" <mkl@...gutronix.de>
To: "Masayuki Ohtake" <masa-korg@....okisemi.com>
Cc: <andrew.chih.howe.khor@...el.com>; <qi.wang@...el.com>; <netdev@...r.kernel.org>; <gregkh@...e.de>;
<yong.y.wang@...el.com>; <socketcan-core@...ts.berlios.de>; "Morinaga" <morinaga526@....okisemi.com>;
<meego-dev@...go.com>; <arjan@...ux.intel.com>; "Wolfgang Grandegger" <wg@...ndegger.com>
Sent: Monday, September 13, 2010 9:29 PM
Subject: Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_CAN driver to 2.6.35


> On 09/13/2010 02:07 PM, Masayuki Ohtake wrote:
> >> - Try to send patches directly with git send-email
> > In our network environment, we can't use send-email.
>
> that's bad...talk to your IT department :)
>
> >> - don't use global variables
> > I have modified so that our patch doesn't use global variable except module parameter.
> >
> >> - don't use that "int handle", e.g.:
> > I have deleted.
> >
> >
> >> - get rid of the intermediate struct pch_can_msg:
> >>    Your data path is:
> >>    struct can_frame ->  struct pch_can_msg ->  registers
> >>    write from struct can_frame into registers directly
> > Since Topcliff CAN HW register assign is different from struct can_frame,
> > I think intermediate structure is necessary.
>
> I don't see any reason for this. No other driver has an intermediate
> struct. Look at the other drivers.
>
> >> - what's the purpose of "p_can_os->can_callback", call the function
> >>    directly from the interrupt handler
> > I have deleted
> >
> >> - implement NAPI
> > Since Topcliff CAN HW register has only single rx buffer,
> > I think NAPI is unnecessary.
>
> Doesn't matter. Please try to implement it.
>
> >> - get rid of "1<<  BIT_SHIFT_SIX" and friend,
> >>    use "1<<  6" or "BIT(6)" if you like defines
> > I have modified.
> >
> >> - use defines to set bits in struct can_frame can_id
> > I have modified.
> >
> > I will resubmit modified our CAN patch soon.
> >
> > Thanks, Ohtake(OKISemi)
>
> cheers, 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   |
>


--
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