linux-kernel - RE: GPIO support for HTC Dream

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]

Date:	Tue, 15 Dec 2009 15:15:14 -0500
From:	"H Hartley Sweeten" <hartleys@...ionengravers.com>
To:	"Pavel Machek" <pavel@....cz>
Cc:	"Ryan Mallon" <ryan@...ewatersys.com>,
	"Daniel Walker" <dwalker@...eaurora.org>,
	"Iliyan Malchev" <malchev@...gle.com>,
	"Brian Swetland" <swetland@...gle.com>,
	"kernel list" <linux-kernel@...r.kernel.org>,
	"Arve Hj?nnev?g" <arve@...roid.com>,
	"linux-arm-kernel" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: GPIO support for HTC Dream

On Tuesday, December 15, 2009 12:47 PM, Pavel Machek wrote:
>
> Hi!
>
>>> +int gpio_to_irq(unsigned gpio)
>>> +{
>>> +	return -EINVAL;
>>> +}
>> 
>> This should probably just be an inline function in
>> arch/arm/mach-msm/include/mach/gpio.h
>
> Well, it is not performance critical in any way and it is likely to
> change in future. I'd leave it here.

Comment below..

>>> diff --git a/arch/arm/mach-msm/board-dream.h b/arch/arm/mach-msm/board-dream.h
>>> index 4f345a5..dbd78b9 100644
>>> --- a/arch/arm/mach-msm/board-dream.h
>>> +++ b/arch/arm/mach-msm/board-dream.h
>>> @@ -1,5 +1,58 @@
>>>  
>>> -#define TROUT_CPLD_BASE   0xE8100000
>>> -#define TROUT_CPLD_START  0x98000000
>>> -#define TROUT_CPLD_SIZE   SZ_4K
>>> +#define MSM_SMI_BASE		0x00000000
>>> +#define MSM_SMI_SIZE		0x00800000
...
>>> +#define DREAM_CPLD_BASE   0xE8100000
>>> +#define DREAM_CPLD_START  0x98000000
>>> +#define DREAM_CPLD_SIZE   SZ_4K
>>> +
>>
>> This header might need to be a separate patch.  The only thing in it
>> related to the rest of this is DREAM_CPLD_BASE.
>
> Yep.
>
>>> +#ifndef __ASM_ARCH_MSM_GPIO_H
>>> +#define __ASM_ARCH_MSM_GPIO_H
>>> +
>>> +extern int gpio_to_irq(unsigned gpio);
>> 
>> This should probably be an inline as mentioned above.
>> 
>> For completeness you should probably also add:
>> 
>> static inline int irq_to_gpio(unsigned irq)
>> {
>> 	return -EINVAL;
>> }
>
> I'd say that would be overdoing it.

I only mentioned that one because it is one of the functions listed in
Documentation/gpio.txt.  I'm not sure if that means it's "required".  You
make the call...

>> And, nitpick, move both of them after the gpio_cansleep below.
>
> I'll do the move.
>
>>> +#define DREAM_GPIO_CABLE_IN1		(83)
>>> +#define DREAM_GPIO_CABLE_IN2		(49)
>>> +
>>> +#define DREAM_GPIO_START (128)
>> 
>> Nitpick.  Tab align these three with the ones below.
>
> Ok.
>
>>> +#define DREAM_GPIO_SDMC_CD_N                (DREAM_GPIO_VIRTUAL_BASE + 0)
>>> +#define DREAM_GPIO_END                      (DREAM_GPIO_SDMC_CD_N)
>>> +#define DREAM_GPIO_BANK1_FIRST_INT_SOURCE   (DREAM_GPIO_SDMC_CD_N)
>>> +#define DREAM_GPIO_BANK1_LAST_INT_SOURCE    (DREAM_GPIO_SDMC_CD_N)
>>> +
>>> +#endif
>> 
>> Otherwise, looks good to me.  Just test it to make sure it works :-).
>> 
>> Since I have no way of compiling or testing this...
>> 
>> Reviewed-by: H Hartley Sweeten <hsweeten@...ionengravers.com>
>
> I believe inlining that function would be bad change. Can I still use
> reviewed-by tag?

The reason I suggest making it an inline in the header is you might
eventually want to change both gpio_to_irq and irq_to_gpio into macros
so that they can be used for static initializers.  For right now both
are simple return -EINVAL but when gpio irq's are supported you might
find a need to use one or the other in a platform init...

Regardless, yes you can still use my reviesed-by tag.

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