Re: [PATCH 1/3] usb: phy: Add RCAR Gen2 USB phy

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2551 bytes --]

Hi,

On Thu, Oct 10, 2013 at 01:21:27AM +0400, Valentine wrote:
> On 10/10/2013 12:32 AM, Laurent Pinchart wrote:
> >Hi Valentine,
> >
> 
> Hi Laurent,
> 
> >Thank you for the patch.
> >
> >On Tuesday 08 October 2013 23:43:25 Valentine Barshak wrote:
> >>This adds RCAR Gen2 USB phy support. The driver configures
> >>USB channels 0/2 which are shared between PCI USB hosts and
> >>USBHS/USBSS devices. It also controls internal USBHS phy.
> >>
> >>Signed-off-by: Valentine Barshak <valentine.barshak@cogentembedded.com>
> >>---
> >>  drivers/usb/phy/Kconfig                         |  13 ++
> >>  drivers/usb/phy/Makefile                        |   1 +
> >>  drivers/usb/phy/phy-rcar-gen2-usb.c             | 255 +++++++++++++++++++++
> >>  include/linux/platform_data/usb-rcar-gen2-phy.h |  22 ++
> >>  4 files changed, 291 insertions(+)
> >>  create mode 100644 drivers/usb/phy/phy-rcar-gen2-usb.c
> >>  create mode 100644 include/linux/platform_data/usb-rcar-gen2-phy.h
> >>
> >>diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
> >>index d5589f9..297062c 100644
> >>--- a/drivers/usb/phy/Kconfig
> >>+++ b/drivers/usb/phy/Kconfig
> >>@@ -214,6 +214,19 @@ config USB_RCAR_PHY
> >>  	  To compile this driver as a module, choose M here: the
> >>  	  module will be called phy-rcar-usb.
> >>
> >>+config USB_RCAR_GEN2_PHY
> >>+	tristate "Renesas R-Car Gen2 USB PHY support"
> >>+	depends on ARCH_R8A7790 || ARCH_R8A7791
> >
> > From a development point of view it's always nice to be able to compile the
> >driver for a wider range of devices, even if the device is only found in the
> >R8A779[01]. This allows catching compilation errors, for instance caused by
> >API changes that affect all drivers using the API being modified.
> 
> Compiling a dirver for an unsupported architecture also seems to be
> more error-prone.

quite the opposite, it helps finding wrong includes and indirect
inclusion mistakes.

> >I would use either
> >
> >	depends on ARM
> >
> >or
> >
> >	depends on ARCH_R8A7790 || ARCH_R8A7791 || COMPILE_TEST
> >
> >(assuming the driver can compile on non-ARM platforms, otherwise the above
> >line could be changed to ARCH_R8A7790 || ARCH_R8A7791 || (ARM &&
> >COMPILE_TEST)).
> 
> OK, I'll take a look.
> Do all the drivers have to support COMPILE_TEST?

new ones, yes. Older ones, just a few. It depends on the subsystem too,
I like to enforce COMPILE_TEST because it helps me build-testing drivers
for which I don't have the HW.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]