[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20120512071727.GA2099@codeaurora.org>
Date: Sat, 12 May 2012 00:17:27 -0700
From: David Brown <davidb@...eaurora.org>
To: Daniel Walker <dwalker@...o99.com>
Cc: Olof Johansson <olof@...om.net>,
Bryan Huntsman <bryanh@...eaurora.org>,
Russell King <linux@....linux.org.uk>,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] arm: msm: fix up very basic HTC Sapphire support
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@...eaurora.org> wrote:
> > From: Daniel Walker <dwalker@...o99.com>
> >
> > Adds sapphire into the make file, and fixes all the code problems that
> > prevented it from building (including adding board-sapphire.h)
> >
> > [davidb@...eaurora.org: Move MACH_TROUT selection back under
> > ARCH_MSM7X00A]
> >
> > Signed-off-by: Daniel Walker <dwalker@...o99.com>
> > Signed-off-by: David Brown <davidb@...eaurora.org>
> > ---
> > 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.
--
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/
Powered by blists - more mailing lists