From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Grandegger Subject: Re: [PATCH net-next-2.6 v2 1/1] can: c_can: Added support for Bosch C_CAN controller Date: Wed, 22 Dec 2010 08:50:42 +0100 Message-ID: <4D11ADD2.40506@grandegger.com> References: <1292407130-19791-1-git-send-email-bhupesh.sharma@st.com> <4D0BD454.3060503@grandegger.com> <4D0FBEF6.8020207@grandegger.com> <4D10FF97.1070703@grandegger.com> <4D11A039.5090006@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org" , "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" To: Marc Kleine-Budde Return-path: In-Reply-To: <4D11A039.5090006-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: socketcan-core-bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org Errors-To: socketcan-core-bounces-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org List-Id: netdev.vger.kernel.org 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. Wolfgang.