[v8,1/2] mfd: add atmel-hlcdc driver

Commit Message

Boris BREZILLON Oct. 6, 2014, 1:48 p.m. UTC

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 <[e-mail]>
Acked-by: Lee Jones <[e-mail]>
Tested-by: Anthony Harivel <[e-mail]>
Tested-by: Ludovic Desroches <[e-mail]>
---
 drivers/mfd/Kconfig             |   6 ++
 drivers/mfd/Makefile            |   1 +
 drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
 4 files changed, 214 insertions(+)
 create mode 100644 drivers/mfd/atmel-hlcdc.c
 create mode 100644 include/linux/mfd/atmel-hlcdc.h

Comments

Lee Jones Oct. 7, 2014, 9:44 a.m. UTC | #1

On Mon, 06 Oct 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 <[e-mail]>
> Acked-by: Lee Jones <[e-mail]>
> Tested-by: Anthony Harivel <[e-mail]>
> Tested-by: Ludovic Desroches <[e-mail]>
> ---
>  drivers/mfd/Kconfig             |   6 ++
>  drivers/mfd/Makefile            |   1 +
>  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
>  4 files changed, 214 insertions(+)
>  create mode 100644 drivers/mfd/atmel-hlcdc.c
>  create mode 100644 include/linux/mfd/atmel-hlcdc.h

Applied for v3.19.

> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index de5abf2..1b925f7 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,12 @@ config MFD_AAT2870_CORE
>  	  additional drivers must be enabled in order to use the
>  	  functionality of the device.
>  
> +config MFD_ATMEL_HLCDC
> +	tristate
> +	select MFD_CORE
> +	select REGMAP_MMIO
> +	depends on OF
> +
>  config MFD_BCM590XX
>  	tristate "Broadcom BCM590xx PMUs"
>  	select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index f001487..df36f68 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -156,6 +156,7 @@ obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
>  obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
>  obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
>  obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
> +obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
>  obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
>  obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
>  obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
> diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
> new file mode 100644
> index 0000000..cfd58f4
> --- /dev/null
> +++ b/drivers/mfd/atmel-hlcdc.c
> @@ -0,0 +1,122 @@
> +/*
> + * Copyright (C) 2014 Free Electrons
> + * Copyright (C) 2014 Atmel
> + *
> + * Author: Boris BREZILLON <[e-mail]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/mfd/atmel-hlcdc.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define ATMEL_HLCDC_REG_MAX		(0x4000 - 0x4)
> +
> +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-display-controller",
> +	},
> +};
> +
> +static const struct regmap_config atmel_hlcdc_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = 4,
> +	.max_register = ATMEL_HLCDC_REG_MAX,
> +};
> +
> +static int atmel_hlcdc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct atmel_hlcdc *hlcdc;
> +	struct resource *res;
> +	void __iomem *regs;
> +
> +	hlcdc = devm_kzalloc(dev, sizeof(*hlcdc), GFP_KERNEL);
> +	if (!hlcdc)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	regs = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(regs))
> +		return PTR_ERR(regs);
> +
> +	hlcdc->irq = platform_get_irq(pdev, 0);
> +	if (hlcdc->irq < 0)
> +		return hlcdc->irq;
> +
> +	hlcdc->periph_clk = devm_clk_get(dev, "periph_clk");
> +	if (IS_ERR(hlcdc->periph_clk)) {
> +		dev_err(dev, "failed to get peripheral clock\n");
> +		return PTR_ERR(hlcdc->periph_clk);
> +	}
> +
> +	hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
> +	if (IS_ERR(hlcdc->sys_clk)) {
> +		dev_err(dev, "failed to get system clock\n");
> +		return PTR_ERR(hlcdc->sys_clk);
> +	}
> +
> +	hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
> +	if (IS_ERR(hlcdc->slow_clk)) {
> +		dev_err(dev, "failed to get slow clock\n");
> +		return PTR_ERR(hlcdc->slow_clk);
> +	}
> +
> +	hlcdc->regmap = devm_regmap_init_mmio(dev, regs,
> +					      &atmel_hlcdc_regmap_config);
> +	if (IS_ERR(hlcdc->regmap))
> +		return PTR_ERR(hlcdc->regmap);
> +
> +	dev_set_drvdata(dev, hlcdc);
> +
> +	return mfd_add_devices(dev, -1, atmel_hlcdc_cells,
> +			       ARRAY_SIZE(atmel_hlcdc_cells),
> +			       NULL, 0, NULL);
> +}
> +
> +static int atmel_hlcdc_remove(struct platform_device *pdev)
> +{
> +	mfd_remove_devices(&pdev->dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id atmel_hlcdc_match[] = {
> +	{ .compatible = "atmel,sama5d3-hlcdc" },
> +	{ /* sentinel */ },
> +};
> +
> +static struct platform_driver atmel_hlcdc_driver = {
> +	.probe = atmel_hlcdc_probe,
> +	.remove = atmel_hlcdc_remove,
> +	.driver = {
> +		.name = "atmel-hlcdc",
> +		.of_match_table = atmel_hlcdc_match,
> +	},
> +};
> +module_platform_driver(atmel_hlcdc_driver);
> +
> +MODULE_ALIAS("platform:atmel-hlcdc");
> +MODULE_AUTHOR("Boris Brezillon <[e-mail]>");
> +MODULE_DESCRIPTION("Atmel HLCDC driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
> new file mode 100644
> index 0000000..1279ab1
> --- /dev/null
> +++ b/include/linux/mfd/atmel-hlcdc.h
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (C) 2014 Free Electrons
> + * Copyright (C) 2014 Atmel
> + *
> + * Author: Boris BREZILLON <[e-mail]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __LINUX_MFD_HLCDC_H
> +#define __LINUX_MFD_HLCDC_H
> +
> +#include <linux/clk.h>
> +#include <linux/regmap.h>
> +
> +#define ATMEL_HLCDC_CFG(i)		((i) * 0x4)
> +#define ATMEL_HLCDC_SIG_CFG		LCDCFG(5)
> +#define ATMEL_HLCDC_HSPOL		BIT(0)
> +#define ATMEL_HLCDC_VSPOL		BIT(1)
> +#define ATMEL_HLCDC_VSPDLYS		BIT(2)
> +#define ATMEL_HLCDC_VSPDLYE		BIT(3)
> +#define ATMEL_HLCDC_DISPPOL		BIT(4)
> +#define ATMEL_HLCDC_DITHER		BIT(6)
> +#define ATMEL_HLCDC_DISPDLY		BIT(7)
> +#define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
> +#define ATMEL_HLCDC_PP			BIT(10)
> +#define ATMEL_HLCDC_VSPSU		BIT(12)
> +#define ATMEL_HLCDC_VSPHO		BIT(13)
> +#define ATMEL_HLCDC_GUARDTIME_MASK	GENMASK(20, 16)
> +
> +#define ATMEL_HLCDC_EN			0x20
> +#define ATMEL_HLCDC_DIS			0x24
> +#define ATMEL_HLCDC_SR			0x28
> +#define ATMEL_HLCDC_IER			0x2c
> +#define ATMEL_HLCDC_IDR			0x30
> +#define ATMEL_HLCDC_IMR			0x34
> +#define ATMEL_HLCDC_ISR			0x38
> +
> +#define ATMEL_HLCDC_CLKPOL		BIT(0)
> +#define ATMEL_HLCDC_CLKSEL		BIT(2)
> +#define ATMEL_HLCDC_CLKPWMSEL		BIT(3)
> +#define ATMEL_HLCDC_CGDIS(i)		BIT(8 + (i))
> +#define ATMEL_HLCDC_CLKDIV_SHFT		16
> +#define ATMEL_HLCDC_CLKDIV_MASK		GENMASK(23, 16)
> +#define ATMEL_HLCDC_CLKDIV(div)		((div - 2) << ATMEL_HLCDC_CLKDIV_SHFT)
> +
> +#define ATMEL_HLCDC_PIXEL_CLK		BIT(0)
> +#define ATMEL_HLCDC_SYNC		BIT(1)
> +#define ATMEL_HLCDC_DISP		BIT(2)
> +#define ATMEL_HLCDC_PWM			BIT(3)
> +#define ATMEL_HLCDC_SIP			BIT(4)
> +
> +#define ATMEL_HLCDC_SOF			BIT(0)
> +#define ATMEL_HLCDC_SYNCDIS		BIT(1)
> +#define ATMEL_HLCDC_FIFOERR		BIT(4)
> +#define ATMEL_HLCDC_LAYER_STATUS(x)	BIT((x) + 8)
> +
> +/**
> + * Structure shared by the MFD device and its subdevices.
> + *
> + * @regmap: register map used to access HLCDC IP registers
> + * @periph_clk: the hlcdc peripheral clock
> + * @sys_clk: the hlcdc system clock
> + * @slow_clk: the system slow clk
> + * @irq: the hlcdc irq
> + */
> +struct atmel_hlcdc {
> +	struct regmap *regmap;
> +	struct clk *periph_clk;
> +	struct clk *sys_clk;
> +	struct clk *slow_clk;
> +	int irq;
> +};
> +
> +#endif /* __LINUX_MFD_HLCDC_H */

Thierry Reding Oct. 7, 2014, 9:47 a.m. UTC | #2

On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> On Mon, 06 Oct 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 <[e-mail]>
> > Acked-by: Lee Jones <[e-mail]>
> > Tested-by: Anthony Harivel <[e-mail]>
> > Tested-by: Ludovic Desroches <[e-mail]>
> > ---
> >  drivers/mfd/Kconfig             |   6 ++
> >  drivers/mfd/Makefile            |   1 +
> >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> >  4 files changed, 214 insertions(+)
> >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> 
> Applied for v3.19.

Will you provide a stable branch that I can pull into the PWM tree?

Thierry

Lee Jones Oct. 7, 2014, 9:59 a.m. UTC | #3

On Tue, 07 Oct 2014, Thierry Reding wrote:

> On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > On Mon, 06 Oct 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 <[e-mail]>
> > > Acked-by: Lee Jones <[e-mail]>
> > > Tested-by: Anthony Harivel <[e-mail]>
> > > Tested-by: Ludovic Desroches <[e-mail]>
> > > ---
> > >  drivers/mfd/Kconfig             |   6 ++
> > >  drivers/mfd/Makefile            |   1 +
> > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > >  4 files changed, 214 insertions(+)
> > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > 
> > Applied for v3.19.
> 
> Will you provide a stable branch that I can pull into the PWM tree?

I hadn't planned on it.  What do you need that for?

Thierry Reding Oct. 7, 2014, 10:06 a.m. UTC | #4

On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> On Tue, 07 Oct 2014, Thierry Reding wrote:
> 
> > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > On Mon, 06 Oct 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 <[e-mail]>
> > > > Acked-by: Lee Jones <[e-mail]>
> > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > ---
> > > >  drivers/mfd/Kconfig             |   6 ++
> > > >  drivers/mfd/Makefile            |   1 +
> > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > >  4 files changed, 214 insertions(+)
> > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > 
> > > Applied for v3.19.
> > 
> > Will you provide a stable branch that I can pull into the PWM tree?
> 
> I hadn't planned on it.  What do you need that for?

Because the PWM driver depends on this series. But if you prefer you
could also take the PWM driver through your tree.

Thierry

Lee Jones Oct. 7, 2014, 10:17 a.m. UTC | #5

On Tue, 07 Oct 2014, Thierry Reding wrote:

> On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > 
> > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > ---
> > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > >  drivers/mfd/Makefile            |   1 +
> > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > >  4 files changed, 214 insertions(+)
> > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > 
> > > > Applied for v3.19.
> > > 
> > > Will you provide a stable branch that I can pull into the PWM tree?
> > 
> > I hadn't planned on it.  What do you need that for?
> 
> Because the PWM driver depends on this series. But if you prefer you
> could also take the PWM driver through your tree.

Probably better to deal with that via Kconfig.

Thierry Reding Oct. 7, 2014, 11:13 a.m. UTC | #6

On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> On Tue, 07 Oct 2014, Thierry Reding wrote:
> 
> > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > 
> > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > ---
> > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > >  4 files changed, 214 insertions(+)
> > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > 
> > > > > Applied for v3.19.
> > > > 
> > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > 
> > > I hadn't planned on it.  What do you need that for?
> > 
> > Because the PWM driver depends on this series. But if you prefer you
> > could also take the PWM driver through your tree.
> 
> Probably better to deal with that via Kconfig.

Do you have any suggestions? The PWM driver currently selects the
MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
if the latter isn't defined.

Thierry

Lee Jones Oct. 7, 2014, 11:38 a.m. UTC | #7

On Tue, 07 Oct 2014, Thierry Reding wrote:

> On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > 
> > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > 
> > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > ---
> > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > >  4 files changed, 214 insertions(+)
> > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > 
> > > > > > Applied for v3.19.
> > > > > 
> > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > 
> > > > I hadn't planned on it.  What do you need that for?
> > > 
> > > Because the PWM driver depends on this series. But if you prefer you
> > > could also take the PWM driver through your tree.
> > 
> > Probably better to deal with that via Kconfig.
> 
> Do you have any suggestions? The PWM driver currently selects the
> MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> if the latter isn't defined.

s/select/depends on/ for the desired effect.

Boris BREZILLON Oct. 7, 2014, 11:41 a.m. UTC | #8

On Tue, 7 Oct 2014 12:38:14 +0100
Lee Jones <[e-mail]> wrote:

> On Tue, 07 Oct 2014, Thierry Reding wrote:
> 
> > On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > 
> > > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > 
> > > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > > ---
> > > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > > >  4 files changed, 214 insertions(+)
> > > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > > 
> > > > > > > Applied for v3.19.
> > > > > > 
> > > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > > 
> > > > > I hadn't planned on it.  What do you need that for?
> > > > 
> > > > Because the PWM driver depends on this series. But if you prefer you
> > > > could also take the PWM driver through your tree.
> > > 
> > > Probably better to deal with that via Kconfig.
> > 
> > Do you have any suggestions? The PWM driver currently selects the
> > MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> > if the latter isn't defined.
> 
> s/select/depends on/ for the desired effect.
> 

Don't forget the atmel-hlcdc.h header file which is referenced by both
the DRM and the PWM drivers.

Thierry Reding Oct. 7, 2014, 11:55 a.m. UTC | #9

On Tue, Oct 07, 2014 at 01:41:12PM +0200, Boris Brezillon wrote:
> On Tue, 7 Oct 2014 12:38:14 +0100
> Lee Jones <[e-mail]> wrote:
> 
> > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > 
> > > On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > 
> > > > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > > 
> > > > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > > > ---
> > > > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > > > >  4 files changed, 214 insertions(+)
> > > > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > > > 
> > > > > > > > Applied for v3.19.
> > > > > > > 
> > > > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > > > 
> > > > > > I hadn't planned on it.  What do you need that for?
> > > > > 
> > > > > Because the PWM driver depends on this series. But if you prefer you
> > > > > could also take the PWM driver through your tree.
> > > > 
> > > > Probably better to deal with that via Kconfig.
> > > 
> > > Do you have any suggestions? The PWM driver currently selects the
> > > MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> > > if the latter isn't defined.
> > 
> > s/select/depends on/ for the desired effect.
> > 
> 
> Don't forget the atmel-hlcdc.h header file which is referenced by both
> the DRM and the PWM drivers.

The depends on will prevent the PWM driver from being built until MFD
becomes available, so the missing header file shouldn't be a problem.

That said, Nicolas Ferre (Cc'ing) at some point requested this to become
a select (or at least for the DRM driver, but I guess the same applies
to PWM) on the grounds that a depends on will make it more difficult to
enable the driver.

So we have two options here: 1) turn the select into a depends on here
and allow the dependency to be resolved that way, or 2) solve the
dependency by making sure the MFD part is merged first (either by
pulling the MFD tree into the PWM and DRM trees or waiting for a full
cycle for the MFD changes to land).

I don't mind either way.

Thierry

Lee Jones Oct. 7, 2014, 11:59 a.m. UTC | #10

On Tue, 07 Oct 2014, Boris Brezillon wrote:

> On Tue, 7 Oct 2014 12:38:14 +0100
> Lee Jones <[e-mail]> wrote:
> 
> > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > 
> > > On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > 
> > > > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > > 
> > > > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > > > ---
> > > > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > > > >  4 files changed, 214 insertions(+)
> > > > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > > > 
> > > > > > > > Applied for v3.19.
> > > > > > > 
> > > > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > > > 
> > > > > > I hadn't planned on it.  What do you need that for?
> > > > > 
> > > > > Because the PWM driver depends on this series. But if you prefer you
> > > > > could also take the PWM driver through your tree.
> > > > 
> > > > Probably better to deal with that via Kconfig.
> > > 
> > > Do you have any suggestions? The PWM driver currently selects the
> > > MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> > > if the latter isn't defined.
> > 
> > s/select/depends on/ for the desired effect.
> > 
> 
> Don't forget the atmel-hlcdc.h header file which is referenced by both
> the DRM and the PWM drivers.

If you use 'depends on' the DRM and PWM drivers won't even attempt to
compile.

Lee Jones Oct. 7, 2014, noon UTC | #11

On Tue, 07 Oct 2014, Lee Jones wrote:

> On Tue, 07 Oct 2014, Boris Brezillon wrote:
> 
> > On Tue, 7 Oct 2014 12:38:14 +0100
> > Lee Jones <[e-mail]> wrote:
> > 
> > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > 
> > > > On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > 
> > > > > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > > > 
> > > > > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > > > > >  4 files changed, 214 insertions(+)
> > > > > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > > > > 
> > > > > > > > > Applied for v3.19.
> > > > > > > > 
> > > > > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > > > > 
> > > > > > > I hadn't planned on it.  What do you need that for?
> > > > > > 
> > > > > > Because the PWM driver depends on this series. But if you prefer you
> > > > > > could also take the PWM driver through your tree.
> > > > > 
> > > > > Probably better to deal with that via Kconfig.
> > > > 
> > > > Do you have any suggestions? The PWM driver currently selects the
> > > > MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> > > > if the latter isn't defined.
> > > 
> > > s/select/depends on/ for the desired effect.
> > > 
> > 
> > Don't forget the atmel-hlcdc.h header file which is referenced by both
> > the DRM and the PWM drivers.

I should probably attempt to finish this sentence.

> If you use 'depends on' the DRM and PWM drivers won't even attempt to
> compile...

... if the MFD driver (and thus the header file) is not present.

Lee Jones Oct. 7, 2014, 12:22 p.m. UTC | #12

On Tue, 07 Oct 2014, Thierry Reding wrote:
> On Tue, Oct 07, 2014 at 01:41:12PM +0200, Boris Brezillon wrote:
> > On Tue, 7 Oct 2014 12:38:14 +0100
> > Lee Jones <[e-mail]> wrote:
> > 
> > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > 
> > > > On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
> > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > 
> > > > > > On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
> > > > > > > On Tue, 07 Oct 2014, Thierry Reding wrote:
> > > > > > > 
> > > > > > > > On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
> > > > > > > > > On Mon, 06 Oct 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 <[e-mail]>
> > > > > > > > > > Acked-by: Lee Jones <[e-mail]>
> > > > > > > > > > Tested-by: Anthony Harivel <[e-mail]>
> > > > > > > > > > Tested-by: Ludovic Desroches <[e-mail]>
> > > > > > > > > > ---
> > > > > > > > > >  drivers/mfd/Kconfig             |   6 ++
> > > > > > > > > >  drivers/mfd/Makefile            |   1 +
> > > > > > > > > >  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
> > > > > > > > > >  4 files changed, 214 insertions(+)
> > > > > > > > > >  create mode 100644 drivers/mfd/atmel-hlcdc.c
> > > > > > > > > >  create mode 100644 include/linux/mfd/atmel-hlcdc.h
> > > > > > > > > 
> > > > > > > > > Applied for v3.19.
> > > > > > > > 
> > > > > > > > Will you provide a stable branch that I can pull into the PWM tree?
> > > > > > > 
> > > > > > > I hadn't planned on it.  What do you need that for?
> > > > > > 
> > > > > > Because the PWM driver depends on this series. But if you prefer you
> > > > > > could also take the PWM driver through your tree.
> > > > > 
> > > > > Probably better to deal with that via Kconfig.
> > > > 
> > > > Do you have any suggestions? The PWM driver currently selects the
> > > > MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
> > > > if the latter isn't defined.
> > > 
> > > s/select/depends on/ for the desired effect.
> > > 
> > 
> > Don't forget the atmel-hlcdc.h header file which is referenced by both
> > the DRM and the PWM drivers.
> 
> The depends on will prevent the PWM driver from being built until MFD
> becomes available, so the missing header file shouldn't be a problem.
> 
> That said, Nicolas Ferre (Cc'ing) at some point requested this to become
> a select (or at least for the DRM driver, but I guess the same applies
> to PWM) on the grounds that a depends on will make it more difficult to
> enable the driver.

It's not that much more difficult.  It just entails enabling 3 instead
of 2 config options.  Once all of the required components are merged,
feel free to drop back to 'select'.  This is easier than sharing round
immutable branches all over the place.

> So we have two options here: 1) turn the select into a depends on here
> and allow the dependency to be resolved that way, or 2) solve the
> dependency by making sure the MFD part is merged first (either by
> pulling the MFD tree into the PWM and DRM trees or waiting for a full
> cycle for the MFD changes to land).
> 
> I don't mind either way.

I'll go with either of the two suggestions above.

Nicolas Ferre Oct. 7, 2014, 1:16 p.m. UTC | #13

Hi Lee,

On 07/10/2014 14:22, Lee Jones :
> On Tue, 07 Oct 2014, Thierry Reding wrote:
>> On Tue, Oct 07, 2014 at 01:41:12PM +0200, Boris Brezillon wrote:
>>> On Tue, 7 Oct 2014 12:38:14 +0100
>>> Lee Jones <[e-mail]> wrote:
>>>
>>>> On Tue, 07 Oct 2014, Thierry Reding wrote:
>>>>
>>>>> On Tue, Oct 07, 2014 at 11:17:43AM +0100, Lee Jones wrote:
>>>>>> On Tue, 07 Oct 2014, Thierry Reding wrote:
>>>>>>
>>>>>>> On Tue, Oct 07, 2014 at 10:59:32AM +0100, Lee Jones wrote:
>>>>>>>> On Tue, 07 Oct 2014, Thierry Reding wrote:
>>>>>>>>
>>>>>>>>> On Tue, Oct 07, 2014 at 10:44:27AM +0100, Lee Jones wrote:
>>>>>>>>>> On Mon, 06 Oct 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 <[e-mail]>
>>>>>>>>>>> Acked-by: Lee Jones <[e-mail]>
>>>>>>>>>>> Tested-by: Anthony Harivel <[e-mail]>
>>>>>>>>>>> Tested-by: Ludovic Desroches <[e-mail]>
>>>>>>>>>>> ---
>>>>>>>>>>>  drivers/mfd/Kconfig             |   6 ++
>>>>>>>>>>>  drivers/mfd/Makefile            |   1 +
>>>>>>>>>>>  drivers/mfd/atmel-hlcdc.c       | 122 ++++++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>  include/linux/mfd/atmel-hlcdc.h |  85 ++++++++++++++++++++++++++++
>>>>>>>>>>>  4 files changed, 214 insertions(+)
>>>>>>>>>>>  create mode 100644 drivers/mfd/atmel-hlcdc.c
>>>>>>>>>>>  create mode 100644 include/linux/mfd/atmel-hlcdc.h
>>>>>>>>>>
>>>>>>>>>> Applied for v3.19.
>>>>>>>>>
>>>>>>>>> Will you provide a stable branch that I can pull into the PWM tree?
>>>>>>>>
>>>>>>>> I hadn't planned on it.  What do you need that for?
>>>>>>>
>>>>>>> Because the PWM driver depends on this series. But if you prefer you
>>>>>>> could also take the PWM driver through your tree.
>>>>>>
>>>>>> Probably better to deal with that via Kconfig.
>>>>>
>>>>> Do you have any suggestions? The PWM driver currently selects the
>>>>> MFD_ATMEL_HLCDC symbol, which as I understand will cause a Kconfig error
>>>>> if the latter isn't defined.
>>>>
>>>> s/select/depends on/ for the desired effect.
>>>>
>>>
>>> Don't forget the atmel-hlcdc.h header file which is referenced by both
>>> the DRM and the PWM drivers.
>>
>> The depends on will prevent the PWM driver from being built until MFD
>> becomes available, so the missing header file shouldn't be a problem.
>>
>> That said, Nicolas Ferre (Cc'ing) at some point requested this to become
>> a select (or at least for the DRM driver, but I guess the same applies
>> to PWM) on the grounds that a depends on will make it more difficult to
>> enable the driver.
> 
> It's not that much more difficult.  It just entails enabling 3 instead
> of 2 config options.

Yes it is more difficult. Believe me, it's a mess, but...

> Once all of the required components are merged,
> feel free to drop back to 'select'.  This is easier than sharing round
> immutable branches all over the place.

.. I agree with this option of moving to an easier-to-merge solution and
then dealing with the ease of use.

>> So we have two options here: 1) turn the select into a depends on here
>> and allow the dependency to be resolved that way, or 2) solve the
>> dependency by making sure the MFD part is merged first (either by
>> pulling the MFD tree into the PWM and DRM trees or waiting for a full
>> cycle for the MFD changes to land).
>>
>> I don't mind either way.
> 
> I'll go with either of the two suggestions above.

So, Lee and Thierry, you can both take your part in your respective
trees with the change (1) described above and with my:

Acked-by: Nicolas Ferre <[e-mail]>

(if you feel it is needed).

Thanks, bye,

Lee Jones Oct. 7, 2014, 1:28 p.m. UTC | #14

> >> That said, Nicolas Ferre (Cc'ing) at some point requested this to become
> >> a select (or at least for the DRM driver, but I guess the same applies
> >> to PWM) on the grounds that a depends on will make it more difficult to
> >> enable the driver.
> > 
> > It's not that much more difficult.  It just entails enabling 3 instead
> > of 2 config options.
> 
> Yes it is more difficult. Believe me, it's a mess, but...
> 
> > Once all of the required components are merged,
> > feel free to drop back to 'select'.  This is easier than sharing round
> > immutable branches all over the place.
> 
> .. I agree with this option of moving to an easier-to-merge solution and
> then dealing with the ease of use.
> 
> >> So we have two options here: 1) turn the select into a depends on here
> >> and allow the dependency to be resolved that way, or 2) solve the
> >> dependency by making sure the MFD part is merged first (either by
> >> pulling the MFD tree into the PWM and DRM trees or waiting for a full
> >> cycle for the MFD changes to land).
> >>
> >> I don't mind either way.
> > 
> > I'll go with either of the two suggestions above.
> 
> So, Lee and Thierry, you can both take your part in your respective
> trees with the change (1) described above and with my:
> 
> Acked-by: Nicolas Ferre <[e-mail]>

Works for me.  The MFD part has already been applied.

Patch

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..1b925f7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -59,6 +59,12 @@  config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_ATMEL_HLCDC
+	tristate
+	select MFD_CORE
+	select REGMAP_MMIO
+	depends on OF
+
 config MFD_BCM590XX
 	tristate "Broadcom BCM590xx PMUs"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..df36f68 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -156,6 +156,7 @@  obj-$(CONFIG_MFD_PM8921_CORE) 	+= pm8921-core.o ssbi.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
diff --git a/drivers/mfd/atmel-hlcdc.c b/drivers/mfd/atmel-hlcdc.c
new file mode 100644
index 0000000..cfd58f4
--- /dev/null
+++ b/drivers/mfd/atmel-hlcdc.c
@@ -0,0 +1,122 @@ 
+/*
+ * Copyright (C) 2014 Free Electrons
+ * Copyright (C) 2014 Atmel
+ *
+ * Author: Boris BREZILLON <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/atmel-hlcdc.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define ATMEL_HLCDC_REG_MAX		(0x4000 - 0x4)
+
+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-display-controller",
+	},
+};
+
+static const struct regmap_config atmel_hlcdc_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = ATMEL_HLCDC_REG_MAX,
+};
+
+static int atmel_hlcdc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct atmel_hlcdc *hlcdc;
+	struct resource *res;
+	void __iomem *regs;
+
+	hlcdc = devm_kzalloc(dev, sizeof(*hlcdc), GFP_KERNEL);
+	if (!hlcdc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(regs))
+		return PTR_ERR(regs);
+
+	hlcdc->irq = platform_get_irq(pdev, 0);
+	if (hlcdc->irq < 0)
+		return hlcdc->irq;
+
+	hlcdc->periph_clk = devm_clk_get(dev, "periph_clk");
+	if (IS_ERR(hlcdc->periph_clk)) {
+		dev_err(dev, "failed to get peripheral clock\n");
+		return PTR_ERR(hlcdc->periph_clk);
+	}
+
+	hlcdc->sys_clk = devm_clk_get(dev, "sys_clk");
+	if (IS_ERR(hlcdc->sys_clk)) {
+		dev_err(dev, "failed to get system clock\n");
+		return PTR_ERR(hlcdc->sys_clk);
+	}
+
+	hlcdc->slow_clk = devm_clk_get(dev, "slow_clk");
+	if (IS_ERR(hlcdc->slow_clk)) {
+		dev_err(dev, "failed to get slow clock\n");
+		return PTR_ERR(hlcdc->slow_clk);
+	}
+
+	hlcdc->regmap = devm_regmap_init_mmio(dev, regs,
+					      &atmel_hlcdc_regmap_config);
+	if (IS_ERR(hlcdc->regmap))
+		return PTR_ERR(hlcdc->regmap);
+
+	dev_set_drvdata(dev, hlcdc);
+
+	return mfd_add_devices(dev, -1, atmel_hlcdc_cells,
+			       ARRAY_SIZE(atmel_hlcdc_cells),
+			       NULL, 0, NULL);
+}
+
+static int atmel_hlcdc_remove(struct platform_device *pdev)
+{
+	mfd_remove_devices(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id atmel_hlcdc_match[] = {
+	{ .compatible = "atmel,sama5d3-hlcdc" },
+	{ /* sentinel */ },
+};
+
+static struct platform_driver atmel_hlcdc_driver = {
+	.probe = atmel_hlcdc_probe,
+	.remove = atmel_hlcdc_remove,
+	.driver = {
+		.name = "atmel-hlcdc",
+		.of_match_table = atmel_hlcdc_match,
+	},
+};
+module_platform_driver(atmel_hlcdc_driver);
+
+MODULE_ALIAS("platform:atmel-hlcdc");
+MODULE_AUTHOR("Boris Brezillon <[e-mail]>");
+MODULE_DESCRIPTION("Atmel HLCDC driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/atmel-hlcdc.h b/include/linux/mfd/atmel-hlcdc.h
new file mode 100644
index 0000000..1279ab1
--- /dev/null
+++ b/include/linux/mfd/atmel-hlcdc.h
@@ -0,0 +1,85 @@ 
+/*
+ * Copyright (C) 2014 Free Electrons
+ * Copyright (C) 2014 Atmel
+ *
+ * Author: Boris BREZILLON <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __LINUX_MFD_HLCDC_H
+#define __LINUX_MFD_HLCDC_H
+
+#include <linux/clk.h>
+#include <linux/regmap.h>
+
+#define ATMEL_HLCDC_CFG(i)		((i) * 0x4)
+#define ATMEL_HLCDC_SIG_CFG		LCDCFG(5)
+#define ATMEL_HLCDC_HSPOL		BIT(0)
+#define ATMEL_HLCDC_VSPOL		BIT(1)
+#define ATMEL_HLCDC_VSPDLYS		BIT(2)
+#define ATMEL_HLCDC_VSPDLYE		BIT(3)
+#define ATMEL_HLCDC_DISPPOL		BIT(4)
+#define ATMEL_HLCDC_DITHER		BIT(6)
+#define ATMEL_HLCDC_DISPDLY		BIT(7)
+#define ATMEL_HLCDC_MODE_MASK		GENMASK(9, 8)
+#define ATMEL_HLCDC_PP			BIT(10)
+#define ATMEL_HLCDC_VSPSU		BIT(12)
+#define ATMEL_HLCDC_VSPHO		BIT(13)
+#define ATMEL_HLCDC_GUARDTIME_MASK	GENMASK(20, 16)
+
+#define ATMEL_HLCDC_EN			0x20
+#define ATMEL_HLCDC_DIS			0x24
+#define ATMEL_HLCDC_SR			0x28
+#define ATMEL_HLCDC_IER			0x2c
+#define ATMEL_HLCDC_IDR			0x30
+#define ATMEL_HLCDC_IMR			0x34
+#define ATMEL_HLCDC_ISR			0x38
+
+#define ATMEL_HLCDC_CLKPOL		BIT(0)
+#define ATMEL_HLCDC_CLKSEL		BIT(2)
+#define ATMEL_HLCDC_CLKPWMSEL		BIT(3)
+#define ATMEL_HLCDC_CGDIS(i)		BIT(8 + (i))
+#define ATMEL_HLCDC_CLKDIV_SHFT		16
+#define ATMEL_HLCDC_CLKDIV_MASK		GENMASK(23, 16)
+#define ATMEL_HLCDC_CLKDIV(div)		((div - 2) << ATMEL_HLCDC_CLKDIV_SHFT)
+
+#define ATMEL_HLCDC_PIXEL_CLK		BIT(0)
+#define ATMEL_HLCDC_SYNC		BIT(1)
+#define ATMEL_HLCDC_DISP		BIT(2)
+#define ATMEL_HLCDC_PWM			BIT(3)
+#define ATMEL_HLCDC_SIP			BIT(4)
+
+#define ATMEL_HLCDC_SOF			BIT(0)
+#define ATMEL_HLCDC_SYNCDIS		BIT(1)
+#define ATMEL_HLCDC_FIFOERR		BIT(4)
+#define ATMEL_HLCDC_LAYER_STATUS(x)	BIT((x) + 8)
+
+/**
+ * Structure shared by the MFD device and its subdevices.
+ *
+ * @regmap: register map used to access HLCDC IP registers
+ * @periph_clk: the hlcdc peripheral clock
+ * @sys_clk: the hlcdc system clock
+ * @slow_clk: the system slow clk
+ * @irq: the hlcdc irq
+ */
+struct atmel_hlcdc {
+	struct regmap *regmap;
+	struct clk *periph_clk;
+	struct clk *sys_clk;
+	struct clk *slow_clk;
+	int irq;
+};
+
+#endif /* __LINUX_MFD_HLCDC_H */