On Wed, 23 Jul 2014, Peter Griffin wrote:

This patch adds the ST glue logic to manage the DWC3 HC
on STiH407 SoC family. It manages the powerdown signal,
and configures the internal glue logic and syscfg registers.

Signed-off-by: Giuseppe Cavallaro <redacted>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/usb/dwc3/Kconfig   |   9 ++
 drivers/usb/dwc3/Makefile  |   1 +
 drivers/usb/dwc3/dwc3-st.c | 338 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 348 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-st.c

[...]

+/*
+ * For all fields in USB2_VBUS_MNGMNT_SEL1
+ * 2?b00 : Override value from Reg 0x30 is selected
+ * 2?b01 : utmiotg_<signal_name> from usb3_top is selected
+ * 2?b10 : pipew_<signal_name> from PIPEW instance is selected
+ * 2?b11 : value is 1'b0
+ */
+#define REG30	0x0
+#define UTMIOTG	0x1
+#define PIPEW	0x2
+#define ZERO	0x3

Possible register values are usually prefixed with something
descriptive which identifies them.

USB2_VBUS_ looks appropriate here.

[...]

+/**
+ * struct st_dwc3 - st-dwc3 driver private structure
+ * @dwc3:		platform device pointer
+ * @dev:		device pointer
+ * @glue_base		ioaddr for the glue registers
+ * @regmap		regmap pointer for getting syscfg
+ * @syscfg_reg_off	usb syscfg control offset
+ * @dr_mode		drd static host/device config
+ * @rstc_pwrdn		rest controller for powerdown signal
+ * @rstc_rst		reset controller for softreset signal

Some of these have ':', some of them don't.  I suggest you standardise
to 'all do'.

+ *

Superflous line in comment.

+ */
+

Superflous '\n'.

Take a look how you did the function headers below.

[...]

+static int st_dwc3_drd_init(struct st_dwc3 *dwc3_data)
+{
+	u32 val;
+	int err;
+
+	err = regmap_read(dwc3_data->regmap, dwc3_data->syscfg_reg_off, &val);
+	if (err)
+		return err;
+
+	switch (dwc3_data->dr_mode) {
+	case USB_DR_MODE_PERIPHERAL:
+		val |= USB_SET_PORT_DEVICE;
+		dev_dbg(dwc3_data->dev, "Configuring as Device\n");
+		break;
+
+	case USB_DR_MODE_HOST:
+		val &= USB_HOST_DEFAULT_MASK;
+		dev_dbg(dwc3_data->dev, "Configuring as Host\n");
+		break;
+
+	default:
+		dev_err(dwc3_data->dev, "Unsupported mode of operation %d\n"
+			, dwc3_data->dr_mode);

',' should be on the line above.

+		return -EINVAL;
+	}
+
+	return regmap_write(dwc3_data->regmap, dwc3_data->syscfg_reg_off, val);
+}

All of this stuff is pretty minor.

Once fixed apply my Ack on the next revision:

Acked-by: Lee Jones <redacted>

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