Re: [PATCH v2] arm: msm: fix up very basic HTC Sapphire support

[Daniel Walker]

What you did wasn't what I was suggesting you do...

Keep your pull request available as it originally was and ill comment on the thread..

Daniel

David Brown <davidb@xxxxxxxxxxxxxx> wrote:

>On Fri, May 11, 2012 at 10:45:52PM -0700, Olof Johansson wrote:
>
>> Sorry, I should have found the below items on the first review and not
>> now on the second one, but see below.
>
>Daniel, I don't mind fixing up minor things, but can you take care of
>the other issues that Olof has brought up.
>
>Thanks,
>David
>
>> On Fri, May 11, 2012 at 5:15 PM, David Brown <davidb@xxxxxxxxxxxxxx> wrote:
>> > From: Daniel Walker <dwalker@xxxxxxxxxx>
>> >
>> > Adds sapphire into the make file, and fixes all the code problems that
>> > prevented it from building (including adding board-sapphire.h)
>> >
>> > [davidb@xxxxxxxxxxxxxx: Move MACH_TROUT selection back under
>> > ARCH_MSM7X00A]
>> >
>> > Signed-off-by: Daniel Walker <dwalker@xxxxxxxxxx>
>> > Signed-off-by: David Brown <davidb@xxxxxxxxxxxxxx>
>> > ---
>> > v2 - Moved MACH_TROUT selection back under the ARCH config target
>> >
>> > Âarch/arm/mach-msm/Kconfig     Â|  Â9 +-
>> > Âarch/arm/mach-msm/Makefile     |  Â1 +
>> > Âarch/arm/mach-msm/board-sapphire.c | Â 18 +--
>> > Âarch/arm/mach-msm/board-sapphire.h | Â224 ++++++++++++++++++++++++++++++++++++
>> > Â4 files changed, 234 insertions(+), 18 deletions(-)
>> > Âcreate mode 100644 arch/arm/mach-msm/board-sapphire.h
>> >
>> > diff --git a/arch/arm/mach-msm/Kconfig b/arch/arm/mach-msm/Kconfig
>> > index 1cd40ad..70eae63 100644
>> > --- a/arch/arm/mach-msm/Kconfig
>> > +++ b/arch/arm/mach-msm/Kconfig
>> > @@ -6,7 +6,7 @@ choice
>> >
>> > Âconfig ARCH_MSM7X00A
>> > Â Â Â Âbool "MSM7x00A / MSM7x01A"
>> > - Â Â Â select MACH_TROUT if !MACH_HALIBUT
>> > + Â Â Â select MACH_TROUT if (!MACH_HALIBUT && !MACH_SAPPHIRE)
>> 
>> Better, thanks!
>> 
>> 
>> > diff --git a/arch/arm/mach-msm/Makefile b/arch/arm/mach-msm/Makefile
>> > index 4ad3969..aff4e5c 100644
>> > --- a/arch/arm/mach-msm/Makefile
>> > +++ b/arch/arm/mach-msm/Makefile
>> > @@ -20,6 +20,7 @@ CFLAGS_scm.o :=$(call as-instr,.arch_extension sec,-DREQUIRES_SEC=1)
>> > Âobj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
>> > Âobj-$(CONFIG_SMP) += headsmp.o platsmp.o
>> >
>> > +obj-$(CONFIG_MACH_SAPPHIRE) += board-sapphire.o devices-msm7x00.o
>> > Âobj-$(CONFIG_MACH_TROUT) += board-trout.o board-trout-gpio.o board-trout-mmc.o devices-msm7x00.o
>> > Âobj-$(CONFIG_MACH_TROUT) += board-trout.o board-trout-gpio.o board-trout-mmc.o board-trout-panel.o devices-msm7x00.o
>> > Âobj-$(CONFIG_MACH_HALIBUT) += board-halibut.o devices-msm7x00.o
>> 
>> Unrelated to this change per se, but it seems like devices-msm7x00.o
>> should be on an obj-$(CONFIG_ARCH_MSM7X00A) statement instead of
>> duplicated for all boards.
>> 
>> Also, the trout line is mostly duplicated, only delta is panel. Looks
>> broken. Both of these things is material for a separate patch though.
>> 
>> > diff --git a/arch/arm/mach-msm/board-sapphire.h b/arch/arm/mach-msm/board-sapphire.h
>> > new file mode 100644
>> > index 0000000..70f925e
>> > --- /dev/null
>> > +++ b/arch/arm/mach-msm/board-sapphire.h
>> > @@ -0,0 +1,224 @@
>> [..]
>> > +void config_sapphire_camera_on_gpios(void);
>> > +void config_sapphire_camera_off_gpios(void);
>> > +int sapphire_get_smi_size(void);
>> > +unsigned int sapphire_get_hwid(void);
>> > +unsigned int sapphire_get_skuid(void);
>> > +unsigned int is_12pin_camera(void);
>> > +int sapphire_is_5M_camera(void);
>> > +int sapphire_gpio_write(struct gpio_chip *chip, unsigned n, unsigned on);
>> 
>> Many (all?) of these functions are not to be found in the code. Please
>> don't add prototypes for code that isn't there.
>> 
>> 
>> -Olof
>
>-- 
>Sent by an employee of the Qualcomm Innovation Center, Inc.
>The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
N‹§²æìr¸›yúèšØb²X¬¶ÇvØ^–)Þ{.nÇ+‰·¥Š{±‘êçzX§¶›¡Ü}©ž²ÆzÚ&j:+v‰¨¾«‘êçzZ+€Ê+zf£¢·hšˆ§~†­†Ûiÿûàz¹®w¥¢¸?™¨è­Ú&¢)ßf”ù^jÇy§m…á@A«a¶Úÿ0¶ìh®å’i