| Message ID | bb60d97fc76b61c2eabef5a02ebd664c0f57ede0.1591867332.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 1bc95972715ab81fd3fa9f5b45ace5bb607af1b5 |
| Headers | show |
| Series | clk: bcm: Add BCM2711 DVP driver | expand |
Hi Maxime, Am 11.06.20 um 11:23 schrieb Maxime Ripard: > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-clk@vger.kernel.org > Cc: devicetree@vger.kernel.org > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- > drivers/clk/bcm/Kconfig | 11 +++- > drivers/clk/bcm/Makefile | 1 +- > drivers/clk/bcm/clk-bcm2711-dvp.c | 120 +++++++++++++++++++++++++++++++- > 3 files changed, 132 insertions(+) > create mode 100644 drivers/clk/bcm/clk-bcm2711-dvp.c > > diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig > index 8c83977a7dc4..784f12c72365 100644 > --- a/drivers/clk/bcm/Kconfig > +++ b/drivers/clk/bcm/Kconfig > @@ -1,4 +1,15 @@ > # SPDX-License-Identifier: GPL-2.0-only > + > +config CLK_BCM2711_DVP > + tristate "Broadcom BCM2711 DVP support" > + depends on ARCH_BCM2835 ||COMPILE_TEST > + depends on COMMON_CLK > + default ARCH_BCM2835 > + select RESET_SIMPLE > + help > + Enable common clock framework support for the Broadcom BCM2711 > + DVP Controller. > + > config CLK_BCM2835 > bool "Broadcom BCM2835 clock support" > depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST > diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile > index 0070ddf6cdd2..edb66b44cb27 100644 > --- a/drivers/clk/bcm/Makefile > +++ b/drivers/clk/bcm/Makefile > @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm281xx.o > obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o > obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o > +obj-$(CONFIG_CLK_BCM2711_DVP) += clk-bcm2711-dvp.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o > obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o > obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o > diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-dvp.c > new file mode 100644 > index 000000000000..84dbc886e303 > --- /dev/null > +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c > @@ -0,0 +1,120 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +// Copyright 2020 Cerno > + > +#include <linux/clk-provider.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/reset-controller.h> > +#include <linux/reset/reset-simple.h> > + > +#define DVP_HT_RPI_SW_INIT 0x04 > +#define DVP_HT_RPI_MISC_CONFIG 0x08 sorry for not noticing this before. Are these defines specific to the Raspberry Pi, because of RPI? Otherwise i like to see this RPI part removed. Regards Stefan
Hi Stefan, On Thu, Jun 11, 2020 at 05:50:30PM +0200, Stefan Wahren wrote: > > diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-dvp.c > > new file mode 100644 > > index 000000000000..84dbc886e303 > > --- /dev/null > > +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c > > @@ -0,0 +1,120 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +// Copyright 2020 Cerno > > + > > +#include <linux/clk-provider.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/reset-controller.h> > > +#include <linux/reset/reset-simple.h> > > + > > +#define DVP_HT_RPI_SW_INIT 0x04 > > +#define DVP_HT_RPI_MISC_CONFIG 0x08 > > sorry for not noticing this before. Are these defines specific to the > Raspberry Pi, because of RPI? I'm not entirely sure to be honest. It's the names that the register have, but it's not clear to me if it's something specific to the RPi itself, or it just means something else entirely. Maxime
On 6/11/2020 9:52 AM, Maxime Ripard wrote: > Hi Stefan, > > On Thu, Jun 11, 2020 at 05:50:30PM +0200, Stefan Wahren wrote: >>> diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-dvp.c >>> new file mode 100644 >>> index 000000000000..84dbc886e303 >>> --- /dev/null >>> +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c >>> @@ -0,0 +1,120 @@ >>> +// SPDX-License-Identifier: GPL-2.0-or-later >>> +// Copyright 2020 Cerno >>> + >>> +#include <linux/clk-provider.h> >>> +#include <linux/module.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/reset-controller.h> >>> +#include <linux/reset/reset-simple.h> >>> + >>> +#define DVP_HT_RPI_SW_INIT 0x04 >>> +#define DVP_HT_RPI_MISC_CONFIG 0x08 >> >> sorry for not noticing this before. Are these defines specific to the >> Raspberry Pi, because of RPI? > > I'm not entirely sure to be honest. It's the names that the register > have, but it's not clear to me if it's something specific to the RPi > itself, or it just means something else entirely. My understanding is that this is a wrapper that was done specifically for the Raspberry Pi usage of that IP block, which is why it has PI in the name, so this looks good correct, and this does match the internal register database name.
Am 11.06.20 um 19:06 schrieb Florian Fainelli: > > On 6/11/2020 9:52 AM, Maxime Ripard wrote: >> Hi Stefan, >> >> On Thu, Jun 11, 2020 at 05:50:30PM +0200, Stefan Wahren wrote: >>>> diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-dvp.c >>>> new file mode 100644 >>>> index 000000000000..84dbc886e303 >>>> --- /dev/null >>>> +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c >>>> @@ -0,0 +1,120 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-or-later >>>> +// Copyright 2020 Cerno >>>> + >>>> +#include <linux/clk-provider.h> >>>> +#include <linux/module.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/reset-controller.h> >>>> +#include <linux/reset/reset-simple.h> >>>> + >>>> +#define DVP_HT_RPI_SW_INIT 0x04 >>>> +#define DVP_HT_RPI_MISC_CONFIG 0x08 >>> sorry for not noticing this before. Are these defines specific to the >>> Raspberry Pi, because of RPI? >> I'm not entirely sure to be honest. It's the names that the register >> have, but it's not clear to me if it's something specific to the RPi >> itself, or it just means something else entirely. > My understanding is that this is a wrapper that was done specifically > for the Raspberry Pi usage of that IP block, which is why it has PI in > the name, so this looks good correct, and this does match the internal > register database name. Okay, i'm fine with that and the whole series. Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
On Thu, 2020-06-11 at 11:23 +0200, Maxime Ripard wrote: > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-clk@vger.kernel.org > Cc: devicetree@vger.kernel.org > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Regards, Nicolas
Quoting Maxime Ripard (2020-06-11 02:23:16) > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-clk@vger.kernel.org > Cc: devicetree@vger.kernel.org > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Applied to clk-next
Quoting Maxime Ripard (2020-06-11 02:23:16) > The HDMI block has a block that controls clocks and reset signals to the > HDMI0 and HDMI1 controllers. > > Let's expose that through a clock driver implementing a clock and reset > provider. > > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Stephen Boyd <sboyd@kernel.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: linux-clk@vger.kernel.org > Cc: devicetree@vger.kernel.org > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- Applied to clk-next
diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig index 8c83977a7dc4..784f12c72365 100644 --- a/drivers/clk/bcm/Kconfig +++ b/drivers/clk/bcm/Kconfig @@ -1,4 +1,15 @@ # SPDX-License-Identifier: GPL-2.0-only + +config CLK_BCM2711_DVP + tristate "Broadcom BCM2711 DVP support" + depends on ARCH_BCM2835 ||COMPILE_TEST + depends on COMMON_CLK + default ARCH_BCM2835 + select RESET_SIMPLE + help + Enable common clock framework support for the Broadcom BCM2711 + DVP Controller. + config CLK_BCM2835 bool "Broadcom BCM2835 clock support" depends on ARCH_BCM2835 || ARCH_BRCMSTB || COMPILE_TEST diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile index 0070ddf6cdd2..edb66b44cb27 100644 --- a/drivers/clk/bcm/Makefile +++ b/drivers/clk/bcm/Makefile @@ -6,6 +6,7 @@ obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm281xx.o obj-$(CONFIG_CLK_BCM_KONA) += clk-bcm21664.o obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o +obj-$(CONFIG_CLK_BCM2711_DVP) += clk-bcm2711-dvp.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835.o obj-$(CONFIG_CLK_BCM2835) += clk-bcm2835-aux.o obj-$(CONFIG_CLK_RASPBERRYPI) += clk-raspberrypi.o diff --git a/drivers/clk/bcm/clk-bcm2711-dvp.c b/drivers/clk/bcm/clk-bcm2711-dvp.c new file mode 100644 index 000000000000..84dbc886e303 --- /dev/null +++ b/drivers/clk/bcm/clk-bcm2711-dvp.c @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// Copyright 2020 Cerno + +#include <linux/clk-provider.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/reset-controller.h> +#include <linux/reset/reset-simple.h> + +#define DVP_HT_RPI_SW_INIT 0x04 +#define DVP_HT_RPI_MISC_CONFIG 0x08 + +#define NR_CLOCKS 2 +#define NR_RESETS 6 + +struct clk_dvp { + struct clk_hw_onecell_data *data; + struct reset_simple_data reset; +}; + +static const struct clk_parent_data clk_dvp_parent = { + .index = 0, +}; + +static int clk_dvp_probe(struct platform_device *pdev) +{ + struct clk_hw_onecell_data *data; + struct resource *res; + struct clk_dvp *dvp; + void __iomem *base; + int ret; + + dvp = devm_kzalloc(&pdev->dev, sizeof(*dvp), GFP_KERNEL); + if (!dvp) + return -ENOMEM; + platform_set_drvdata(pdev, dvp); + + dvp->data = devm_kzalloc(&pdev->dev, + struct_size(dvp->data, hws, NR_CLOCKS), + GFP_KERNEL); + if (!dvp->data) + return -ENOMEM; + data = dvp->data; + + base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); + if (IS_ERR(base)) + return PTR_ERR(base); + + dvp->reset.rcdev.owner = THIS_MODULE; + dvp->reset.rcdev.nr_resets = NR_RESETS; + dvp->reset.rcdev.ops = &reset_simple_ops; + dvp->reset.rcdev.of_node = pdev->dev.of_node; + dvp->reset.membase = base + DVP_HT_RPI_SW_INIT; + spin_lock_init(&dvp->reset.lock); + + ret = devm_reset_controller_register(&pdev->dev, &dvp->reset.rcdev); + if (ret) + return ret; + + data->hws[0] = clk_hw_register_gate_parent_data(&pdev->dev, + "hdmi0-108MHz", + &clk_dvp_parent, 0, + base + DVP_HT_RPI_MISC_CONFIG, 3, + CLK_GATE_SET_TO_DISABLE, + &dvp->reset.lock); + if (IS_ERR(data->hws[0])) + return PTR_ERR(data->hws[0]); + + data->hws[1] = clk_hw_register_gate_parent_data(&pdev->dev, + "hdmi1-108MHz", + &clk_dvp_parent, 0, + base + DVP_HT_RPI_MISC_CONFIG, 4, + CLK_GATE_SET_TO_DISABLE, + &dvp->reset.lock); + if (IS_ERR(data->hws[1])) { + ret = PTR_ERR(data->hws[1]); + goto unregister_clk0; + } + + data->num = NR_CLOCKS; + ret = of_clk_add_hw_provider(pdev->dev.of_node, of_clk_hw_onecell_get, + data); + if (ret) + goto unregister_clk1; + + return 0; + +unregister_clk1: + clk_hw_unregister_gate(data->hws[1]); + +unregister_clk0: + clk_hw_unregister_gate(data->hws[0]); + return ret; +}; + +static int clk_dvp_remove(struct platform_device *pdev) +{ + struct clk_dvp *dvp = platform_get_drvdata(pdev); + struct clk_hw_onecell_data *data = dvp->data; + + clk_hw_unregister_gate(data->hws[1]); + clk_hw_unregister_gate(data->hws[0]); + + return 0; +} + +static const struct of_device_id clk_dvp_dt_ids[] = { + { .compatible = "brcm,brcm2711-dvp", }, + { /* sentinel */ } +}; + +static struct platform_driver clk_dvp_driver = { + .probe = clk_dvp_probe, + .remove = clk_dvp_remove, + .driver = { + .name = "brcm2711-dvp", + .of_match_table = clk_dvp_dt_ids, + }, +}; +module_platform_driver(clk_dvp_driver);