Bhupesh SHARMA wrote: > Hi Wolfgang, > > Please ignore the earlier mail.. > >> -----Original Message----- >> From: Wolfgang Grandegger [mailto:[email protected]] >> Sent: Wednesday, September 01, 2010 2:44 PM >> To: Bhupesh SHARMA >> Cc: [email protected] >> Subject: Re: [RFC PATCH 1/4] SPEAr320 CCAN driver >> >> Hi Bhupesh, >> >> On 09/01/2010 11:01 AM, Bhupesh SHARMA wrote: >>> Hi Wolfgang, >>> >>>> -----Original Message----- >>>> From: Wolfgang Grandegger [mailto:[email protected]] >>>> Sent: Wednesday, September 01, 2010 12:56 PM >>>> To: Bhupesh SHARMA >>>> Cc: [email protected] >>>> Subject: Re: [RFC PATCH 1/4] SPEAr320 CCAN driver >>>> >>>> Hi Bhupesh, >>>> >>>> On 09/01/2010 06:40 AM, Bhupesh SHARMA wrote: >> ... >> >>>>> It is also particularly useful as we now have a new SPEAr1310 SoC >>>> which also uses the Bosch CCAN controller >>>>> , but the glue logic has changed from SPEAr320 and hence we can >>>> handle most of the platform/machine specific details >>>>> in SPEAr1310 driver and keep Bosch CCAN driver as it is. >>>> What has changed with the new Soc and what platform specific details >>>> and >>>> initializations do you think about? I personally do not see a lot of >>>> platform specific code in your spear320_can.c. Just >> spear320_can_read() >>>> and spear320_can_write() and also these functions look pretty >> generic. >>>> Also note that platform specific initialization is usually not done >> in >>>> the CAN driver but the platform code. >>> In a way you are correct: mainly the spear320_can_read() and >> spear320_can_write() >>> change from spear1310_can_read() and spear1310_can_write() >> implementations. >> >> Can you show us the new spear1310_can_read/write functions? I want to >> understand how generic they really are. > > spear1310 differs from spear320 in the way that 16-bit registers are aligned > at a 16-bit boundary > whereas in spear320 these 16-bit registers are aligned at a 32-bit boundary. > So, > while spear1310 routines should look like this: > > static u16 spear1310_can_read_reg(const struct bosch_ccan_priv *priv, > enum ccan_regs reg) > { > u16 val; > > /* 16 bit registers are aligned at 16-bit boundary */ > val = readw(priv->reg_base + reg); > return val; > } > > static void spear1310_can_write_reg(const struct bosch_ccan_priv *priv, > enum ccan_regs reg, u16 val) > { > /* 16 bit registers are aligned at 16-bit boundary */ > writew(val, priv->reg_base + reg); > } > > The one for SPEAr320 will look like: > static u16 spear320_can_read_reg(const struct bosch_ccan_priv *priv, > enum ccan_regs reg) > { > u16 val; > > /* shifting 1 place because 16 bit registers are word aligned */ > val = readw(priv->reg_base + (reg << 1)); > return val; > } > > static void spear320_can_write_reg(const struct bosch_ccan_priv *priv, > enum ccan_regs reg, u16 val) > { > /* shifting 1 place because 16 bit registers are word aligned */ > writew(val, priv->reg_base + (reg << 1)); > }
[...] > Please let me know on basis of the above register definitions. Have a look at the sja1000_platform driver that Wolfgang already mentioned: http://lxr.linux.no/linux+v2.6.35/drivers/net/can/sja1000/sja1000_platform.c#L39 IMHO for normal SOC like your spears it would be sufficient to have a generic ccan_platform driver, just like the sja1000. If you have a broken ccan integration in your SOC, like on the hynix chip (the one we developed a driver for at pengutronix), you can provide your own driver with the sepcial read/write functions. Does the generic ccan driver support clock handling and does it have support for transceiver enable/disable callbacks? 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 |
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
