netdev - Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH

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: <4CC813B9.3080506@grandegger.com>
Date:	Wed, 27 Oct 2010 13:57:45 +0200
From:	Wolfgang Grandegger <wg@...ndegger.com>
To:	Tomoya MORINAGA <tomoya-linux@....okisemi.com>
CC:	Marc Kleine-Budde <mkl@...gutronix.de>,
	andrew.chih.howe.khor@...el.com, masa-korg@....okisemi.com,
	sameo@...ux.intel.com, margie.foster@...el.com,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	yong.y.wang@...el.com, socketcan-core@...ts.berlios.de,
	kok.howg.ewe@...el.com, joel.clark@...el.com,
	morinaga526@....okisemi.com, David Miller <davem@...emloft.net>,
	chripell@...e.org, qi.wang@...el.com
Subject: Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Fix	buildwarnings

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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html