Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix buildwarnings

[Wolfgang Grandegger]

On 10/27/2010 01:27 PM, Tomoya MORINAGA wrote:
> On Wednesday, October 27, 2010 3:52 AM :  Marc Kleine-Budde and Wolfgang Grandegge wrote:
> 
> The following is some inarticulate points I have for your questions.
> Please give me more information.
> 
>> Do I understand your code correctly? You have a big loop, but only do
>>  two different things at certain values of the loop? Smells fishy.
> Uh, I can't understand your intention.
> Please show in detail.
> This processing does configuration for all message objects.

Not all, but just a few of them. We believe it can be implemented more
efficiently.

> 
>> what does this loop do? why is it nessecarry? I don't like delay loops
>>   in the hot path of a driver.
> This loop is for waiting for all tx Message Object completion.
> This is Topcliff CAN HW specification.

What do you mean with "tx Message Object completion"? Is TX done not
signaled via interrupt? Please explain why you need to wait multiples of
500us here in the hot TX path.

>> If you figured out how to use the endianess conversion functions from
>> the cpu_to_{le,be}-{le,to}_to_cpup family use them here, too.
> Uh,le32_to_cpu have been used already here.
> I can't understand your intention.
> Please show in detail.
> 
> 
>>> All these check if busy in the code make me a bit nervous, can you
>>> please explain why they are needed. A pointer to the manual is okay, too.
>> Me too. I already ask in my previous mail how long that functions
>> usually blocks.
> When accessing read/write from/to Message RAM,
> Since it takes much time for transferring between Register and Message RAM,

Much time means how many mirco-seconds?

> SW must check busy flag of CAN register.
> This is a Topcliff HW specification.

Maybe the busy check could also be done *before* the Message RAM is
accessed to avoid (or minimize) waiting.

>> is there some pdev->name instead of KBUILD_MODNAME that can be used?
> I can't understand your intention.
> pdev(struct pci_dev) doesn't have "name" member. 
> Please show in detail.

Using KBUILD_MODNAME is OK. It does hold the name of the driver as required.

Wolfgang.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/