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 <peppe.cavallaro@xxxxxx>
> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
> ---
> 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 <lee.jones@xxxxxxxxxx>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
