From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Brown Subject: Re: [PATCH v2] arm: msm: fix up very basic HTC Sapphire support Date: Sat, 12 May 2012 22:56:12 -0700 Message-ID: <20120513055612.GA7129@codeaurora.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Daniel Walker Cc: Russell King , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, Bryan Huntsman , Olof Johansson , linux-arm-kernel@lists.infradead.org List-Id: linux-arm-msm@vger.kernel.org On Sat, May 12, 2012 at 09:47:44PM -0700, Daniel Walker wrote: > 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.. Please send out updated patches when you've addressed the issues, and I'll gladly pull them in. As for now, Olof has already pulled the tree without the Sapphire changes. David > Daniel > = > David Brown 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 w= rote: > >> > From: Daniel Walker > >> > > >> > Adds sapphire into the make file, and fixes all the code problems th= at > >> > prevented it from building (including adding board-sapphire.h) > >> > > >> > [davidb@codeaurora.org: Move MACH_TROUT selection back under > >> > ARCH_MSM7X00A] > >> > > >> > Signed-off-by: Daniel Walker > >> > Signed-off-by: David Brown > >> > --- > >> > v2 - Moved MACH_TROUT selection back under the ARCH config target > >> > > >> > =A0arch/arm/mach-msm/Kconfig =A0 =A0 =A0 =A0 =A0| =A0 =A09 +- > >> > =A0arch/arm/mach-msm/Makefile =A0 =A0 =A0 =A0 | =A0 =A01 + > >> > =A0arch/arm/mach-msm/board-sapphire.c | =A0 18 +-- > >> > =A0arch/arm/mach-msm/board-sapphire.h | =A0224 +++++++++++++++++++++= +++++++++++++++ > >> > =A04 files changed, 234 insertions(+), 18 deletions(-) > >> > =A0create 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 > >> > > >> > =A0config ARCH_MSM7X00A > >> > =A0 =A0 =A0 =A0bool "MSM7x00A / MSM7x01A" > >> > - =A0 =A0 =A0 select MACH_TROUT if !MACH_HALIBUT > >> > + =A0 =A0 =A0 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 :=3D$(call as-instr,.arch_extension s= ec,-DREQUIRES_SEC=3D1) > >> > =A0obj-$(CONFIG_HOTPLUG_CPU) +=3D hotplug.o > >> > =A0obj-$(CONFIG_SMP) +=3D headsmp.o platsmp.o > >> > > >> > +obj-$(CONFIG_MACH_SAPPHIRE) +=3D board-sapphire.o devices-msm7x00.o > >> > =A0obj-$(CONFIG_MACH_TROUT) +=3D board-trout.o board-trout-gpio.o bo= ard-trout-mmc.o devices-msm7x00.o > >> > =A0obj-$(CONFIG_MACH_TROUT) +=3D board-trout.o board-trout-gpio.o bo= ard-trout-mmc.o board-trout-panel.o devices-msm7x00.o > >> > =A0obj-$(CONFIG_MACH_HALIBUT) +=3D 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, unsigne= d 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 Foru= m. > > -- = Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.