Re: [PATCH] mfd: cros_ec: allow building for ARM64

[Lee Jones]

On Mon, 11 Apr 2016, Javier Martinez Canillas wrote:
> On 04/11/2016 12:42 PM, Brian Norris wrote:
> > On Mon, Apr 11, 2016 at 09:48:49AM +0100, Lee Jones wrote:
> >> When submitting patches, please use the format appropriate for the
> >> subsystem.
> >>
> >>   `git log --oneline -- <subsystem>` helps with this.
> > 
> > $ git log --oneline --no-merges drivers/mfd/cros_ec*.c | head -10
> > 8827a642a463 mfd: cros_ec_spi: Repair comparison ordering issue
> > 2756db6c6366 mfd: cros_ec_i2c: Fix trivial 'tabs before spaces'
> > whitespace issue.
> > 6d6e44a95316 mfd: cros ec: Lock the SPI bus while holding chipselect
> > 3821a065f567 spi: Drop owner assignment from spi_drivers
> > 385c0012dfa0 mfd: cros_ec_i2c: Add OF match table
> > a78ea195f77a mfd: cros_ec: spi: Add OF match table
> > 0e777366fb0e mfd: Drop owner assignment from i2c_drivers
> > cf649e00769a mfd: cros_ec: Staticise some newly introduced structures
> > ff4378f4b813 mfd: cros_ec: spi: Add delay for asserting CS
> > 57b33ff077be mfd: cros_ec: Support multiple EC in a system
> > 
> > So...what's the problem with the subject?
> >
> 
> I also don't see what's wrong with the subject line...

Then you're not looking hard enough.

Every single patch description in the MFD subsystem starts with an
uppercase character, just as you would any sentence.

> >> Also please provide a commit log containing a short description of the
> >> change.  Are you enabling ARM64 becuase the driver is able to be used
> >> on a new platform?  If not, how did this work before?  Etc.
> > 
> > I can add a short description. But there's really not much more to it
> > than the subject. We have ARM64 platforms we want to use this on. Some
> > are already released, and some are in development.
> >
> 
> I think at least a short description is always welcomed. Even if the
> patch is obvious, it should state why the change was introduced.

+1

> >>> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> >>> ---
> >>>  drivers/mfd/Kconfig | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >>> index eea61e349e26..1d63c5fb3f3c 100644
> >>> --- a/drivers/mfd/Kconfig
> >>> +++ b/drivers/mfd/Kconfig
> >>> @@ -134,7 +134,7 @@ config MFD_CROS_EC
> >>>  	select MFD_CORE
> >>>  	select CHROME_PLATFORMS
> >>>  	select CROS_EC_PROTO
> >>> -	depends on X86 || ARM || COMPILE_TEST
> >>> +	depends on X86 || ARM || ARM64 || COMPILE_TEST
> >>>  	help
> >>>  	  If you say Y here you get support for the ChromeOS Embedded
> >>>  	  Controller (EC) providing keyboard, battery and power services.
> 
> Patch looks good to me, so after resending with a commit message:
> 
> Reviewed-by: Javier Martinez Canillas <javier@xxxxxxxxxxxxxxx>
> 
> Best regards,

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog