netdev - RE: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch C

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]

Message-ID: <D5ECB3C7A6F99444980976A8C6D896384DEAE4855F@EAPEX1MAIL1.st.com>
Date:	Wed, 22 Dec 2010 16:03:32 +0800
From:	Bhupesh SHARMA <bhupesh.sharma@...com>
To:	Wolfgang Grandegger <wg@...ndegger.com>,
	Marc Kleine-Budde <mkl@...gutronix.de>
Cc:	"Socketcan-core@...ts.berlios.de" <Socketcan-core@...ts.berlios.de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch
 C_CAN controller

Hi Marc and Wolfgang,

> -----Original Message-----
> From: Wolfgang Grandegger [mailto:wg@...ndegger.com]
> Sent: Wednesday, December 22, 2010 1:21 PM
> To: Marc Kleine-Budde
> Cc: Bhupesh SHARMA; Socketcan-core@...ts.berlios.de;
> netdev@...r.kernel.org
> Subject: Re: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for
> Bosch C_CAN controller
> 
> On 12/22/2010 07:52 AM, Marc Kleine-Budde wrote:
> > On 12/22/2010 04:36 AM, Bhupesh SHARMA wrote:
> >> Hi Wolfgang,
> >>
> >>> Hi Bhupesh,
> >>>
> >>> On 12/21/2010 05:48 AM, Bhupesh SHARMA wrote:
> >>>> Hi Wolfgang,
> >>> ...
> >>>>> In the meantime I compared the CAN chapter of the PCH manual with
> >>> the
> >>>>> C_CAN manual. The paragraphs I checked are *identical*. This
> makes
> >>>>> clear, that the "pch_can" is a clone of the  C_CAN CAN
> controller,
> >>> with
> >>>>> a few extensions, though. Therefore it would make sense, to
> >>> implement a
> >>>>> bus sensitive interface like for the SJA1000 allowing to handle
> both
> >>>>> CAN
> >>>>> controllers with one driver sooner than later. Therefore, could
> you
> >>>>> please implement:
> >>>>>
> >>>>>   drivers/net/can/c_can/c_can.c
> >>>>>                        /c_can_platform.c
> >>>>>
> >>>>> Then an interface to the PCI based PCH CAN controller could be
> added
> >>>>> easily, e.g. as "pch_pci.c". You already had something similar in
> >>> your
> >>>>> RFC version of the patch, IIRC.
> >>>>
> >>>> This was the approach I initially proposed in my RFC V1 patch :)
> >>>> But unfortunately we could not agree to it.
> >>>
> >>> I know. But at that time I was not aware of any other bus used for
> the
> >>> C_CAN controller.
> >>>
> >>>> So, please let me reiterate what I understood and what was present
> >>>> in RFC version of the patch. Please add your comments/views:
> >>>>
> >>>>         - drivers/net/can/c_can/c_can.c (similar on lines of
> >>> sja1000.c)
> >>>>         i.e. a)no *probe* / *remove* functions here,
> >>>>              b)register read/write implemented here.
> >>>>
> >>>>         - drivers/net/can/c_can/c_can_platform.c (similar on lines
> of
> >>> sja1000_platform.c)
> >>>>         i.e. *probe* / *remove* implemented here,
> >>>
> >>> Yes, that's what I'm thinking about.
> >>>
> >>>> Marc and Tomoya can also add their suggestions so that I can
> finalize
> >>> V3 a.s.a.p.
> >>>
> >>> That would be nice, indeed. Also have a look to Tomoya's PCH
> driver,
> >>> which also looks very good in the meantime.
> >>
> >> I am having a look at Tomoya's PCH driver, but as I mentioned in
> >> RFC V1 patch, I would rather like to have a bus sensitive `c_can`
> driver
> >
> > What do you mean by a "bus sensitive" driver?
> 
> I was thinking about a "bus independent interface" like for the
> SJA1000.
> A bus sensitive driver would the be in c_can_platform.c.
> 
> >> on top of which we can have the platform driver `c_can_platform`
> which
> >> essentially caters to the details of registers mapping/arch
> differences.
> >> Any other functionality like USB/PCI should be present in a separate
> file
> >> like `usb_c_can.c` or `pci_c_can.c`
> >
> > Sounds like the sja1000 approach, which is a good choice.
> 
> I fully agree.
> 
> >> If you agree I will try to circulate V3 a.s.ap.
> >
> > go ahead.
> >
> 
> Yes, please.

Ok, so I would try to circulate V3 by tomorrow with a sja1000 *like* approach that ensures 
in-order packet reception as well.

Regards,
Bhupesh
--
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