Re: [PATCH 3/3] can: c_can: Add support for Bosch D_CAN controller

[Wolfgang Grandegger <wg@grandegger.com>]

On 04/24/2012 10:10 AM, Marc Kleine-Budde wrote:
> On 04/20/2012 11:58 AM, AnilKumar Ch wrote:
>> This patch adds the support for D_CAN controller driver to the existing
>> C_CAN driver.
>>
>> Bosch D_CAN controller is a full-CAN implementation which is compliant
>> to CAN protocol version 2.0 part A and B. Bosch D_CAN user manual can be
>> obtained from: http://www.semiconductors.bosch.de/media/en/pdf/
>> ipmodules_1/can/d_can_users_manual_111.pdf
>>
>> The following are the design choices made while adding D_CAN to C_CAN
>> driver:
>> A new overlay structure is added for d_can controller and care is taken
>> to make sure its member names match with equavalent c_can structure
>> members (even if the d_can specification calls them slightly differently).
>> Note that d_can controller has more registers, so structure members of
>> d_can are more than those in c_can.
>>
>> A new set if read/write macros are used to access the registers common
>> between c_can and d_can. To get the basic d_can functionality working
>> it is sufficient to access just these registers.
> 
> I don't like macros. I've two further possible solutions:

Yes, I don't like that part either, also because of the

  "if (priv->dev_type == DEV_TYPE_D_CAN)"

for each read/write access.

> a) Access the registers via an array. The array index is a "virtual"
>    register, the array's value the physical offset within the c_can or
>    d_can controller.

I was thinking about that as well but using absolute addresses. This
would avoid further calculations for 16/32 bit aligned accesses.

> b) AFAICS you need more than three registers to get the CAN core
>    working. Another possibility is to implement an accessor function
>   for each register.

... offsetof() might be useful for this approach.

> Other, hopefully better, solutions are open for discussion.

The solutions above are not really elegant, but so far I do not have
better ideas.

Wolfgang.