Re: [PATCH v3 01/11] mfd: add atmel-hlcdc driver

[Lee Jones <lee.jones@linaro.org>]

On Mon, 07 Jul 2014, Boris BREZILLON wrote:

> The HLCDC IP available on some Atmel SoCs (i.e. at91sam9n12, at91sam9x5
> family or sama5d3 family) exposes 2 subdevices:
> - a display controller (controlled by a DRM driver)
> - a PWM chip
> 
> The MFD device provides a regmap and several clocks (those connected
> to this hardware block) to its subdevices.
> 
> This way concurrent accesses to the iomem range are handled by the regmap
> framework, and each subdevice can safely access HLCDC registers.
> 
> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com>
> ---
>  drivers/mfd/Kconfig             |  12 ++++
>  drivers/mfd/Makefile            |   1 +
>  drivers/mfd/atmel-hlcdc.c       | 119 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/atmel-hlcdc.h |  78 ++++++++++++++++++++++++++
>  4 files changed, 210 insertions(+)
>  create mode 100644 drivers/mfd/atmel-hlcdc.c
>  create mode 100644 include/linux/mfd/atmel-hlcdc.h

[...]

> +static const struct mfd_cell atmel_hlcdc_cells[] = {
> +	{
> +		.name = "atmel-hlcdc-pwm",
> +		.of_compatible = "atmel,hlcdc-pwm",
> +	},
> +	{
> +		.name = "atmel-hlcdc-dc",
> +		.of_compatible = "atmel,hlcdc-dc",

'dc' is a little ambiguous.

Would be more transparent if s/dc/disp or similar.

> +	},
> +};

[...]

> +static struct platform_driver atmel_hlcdc_driver = {
> +	.probe = atmel_hlcdc_probe,
> +	.remove = atmel_hlcdc_remove,
> +	.driver = {
> +		.name = "atmel-hlcdc",
> +		.owner = THIS_MODULE,

Remove this line, it's not required.

> +		.of_match_table = atmel_hlcdc_match,
> +	},
> +};
> +module_platform_driver(atmel_hlcdc_driver);

Once those minor points are addressed, resubmit with my:

Acked-by: Lee Jones <lee.jones@linaro.org>

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