LKML Archive mirror
 help / color / mirror / Atom feed
From: Chen-Yu Tsai <wens@csie.org>
To: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: Mike Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@codeaurora.org>,
	David Airlie <airlied@linux.ie>,
	Thierry Reding <thierry.reding@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	devicetree <devicetree@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	linux-sunxi <linux-sunxi@googlegroups.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Chen-Yu Tsai <wens@csie.org>, Hans de Goede <hdegoede@redhat.com>,
	Alexander Kaplan <alex@nextthing.co>,
	Boris Brezillon <boris.brezillon@free-electrons.com>,
	Wynter Woods <wynter@nextthing.co>,
	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	Rob Clark <robdclark@gmail.com>, Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH v2 05/26] clk: sunxi: Add display and TCON0 clocks driver
Date: Sat, 16 Jan 2016 23:29:41 +0800	[thread overview]
Message-ID: <CAGb2v65=EVTQo8KFMECVR4=94i0rt0V1H07hWKyXShSZxGYKEQ@mail.gmail.com> (raw)
In-Reply-To: <1452785109-6172-6-git-send-email-maxime.ripard@free-electrons.com>

Hi,

On Thu, Jan 14, 2016 at 11:24 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The A10 SoCs and its relatives has a special clock controller to drive the
> display engines (both frontend and backend), that have a lot in common with
> the clock to drive the first TCON channel.
>
> Add a driver to support both.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/clock/sunxi.txt |   2 +
>  drivers/clk/sunxi/Makefile                        |   1 +
>  drivers/clk/sunxi/clk-sun4i-display.c             | 241 ++++++++++++++++++++++
>  3 files changed, 244 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-sun4i-display.c
>
> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt
> index 8a47b77abfca..5360554a7d3f 100644
> --- a/Documentation/devicetree/bindings/clock/sunxi.txt
> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt
> @@ -55,6 +55,7 @@ Required properties:
>         "allwinner,sun9i-a80-apb1-gates-clk" - for the APB1 gates on A80
>         "allwinner,sun6i-a31-apb2-gates-clk" - for the APB2 gates on A31
>         "allwinner,sun8i-a23-apb2-gates-clk" - for the APB2 gates on A23
> +       "allwinner,sun4i-a10-display-clk" - for the display clocks on the A10
>         "allwinner,sun5i-a13-mbus-clk" - for the MBUS clock on A13
>         "allwinner,sun4i-a10-mmc-clk" - for the MMC clock
>         "allwinner,sun9i-a80-mmc-clk" - for mmc module clocks on A80
> @@ -64,6 +65,7 @@ Required properties:
>         "allwinner,sun8i-a23-mbus-clk" - for the MBUS clock on A23
>         "allwinner,sun7i-a20-out-clk" - for the external output clocks
>         "allwinner,sun7i-a20-gmac-clk" - for the GMAC clock module on A20/A31
> +       "allwinner,sun4i-a10-tcon-ch0-clk" - for the TCON channel 0 clock on the A10
>         "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20
>         "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13
>         "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index cb4c299214ce..a991cd8ca509 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -10,6 +10,7 @@ obj-y += clk-a10-pll2.o
>  obj-y += clk-a20-gmac.o
>  obj-y += clk-mod0.o
>  obj-y += clk-simple-gates.o
> +obj-y += clk-sun4i-display.o
>  obj-y += clk-sun8i-mbus.o
>  obj-y += clk-sun9i-core.o
>  obj-y += clk-sun9i-mmc.o
> diff --git a/drivers/clk/sunxi/clk-sun4i-display.c b/drivers/clk/sunxi/clk-sun4i-display.c
> new file mode 100644
> index 000000000000..9dc6894f0934
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> @@ -0,0 +1,241 @@
> +/*
> + * Copyright 2015 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/of_address.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +struct sun4i_a10_display_clk_data {
> +       bool    has_div;
> +       bool    has_rst;
> +       u8      parents;
> +
> +       u8      offset_en;
> +       u8      offset_div;
> +       u8      offset_mux;
> +       u8      offset_rst;
> +
> +       u8      width_div;
> +       u8      width_mux;
> +};
> +
> +struct reset_data {
> +       void __iomem                    *reg;
> +       spinlock_t                      *lock;
> +       struct reset_controller_dev     rcdev;
> +       u8                              offset;
> +};
> +
> +static DEFINE_SPINLOCK(sun4i_a10_display_lock);
> +
> +static inline struct reset_data *rcdev_to_reset_data(struct reset_controller_dev *rcdev)
> +{
> +       return container_of(rcdev, struct reset_data, rcdev);
> +};
> +
> +static int sun4i_a10_display_assert(struct reset_controller_dev *rcdev,
> +                                   unsigned long id)
> +{
> +       struct reset_data *data = rcdev_to_reset_data(rcdev);
> +       unsigned long flags;
> +       u32 reg;
> +
> +       spin_lock_irqsave(data->lock, flags);
> +
> +       reg = readl(data->reg);
> +       writel(reg & ~BIT(data->offset), data->reg);
> +
> +       spin_unlock_irqrestore(data->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int sun4i_a10_display_deassert(struct reset_controller_dev *rcdev,
> +                                     unsigned long id)
> +{
> +       struct reset_data *data = rcdev_to_reset_data(rcdev);
> +       unsigned long flags;
> +       u32 reg;
> +
> +       spin_lock_irqsave(data->lock, flags);
> +
> +       reg = readl(data->reg);
> +       writel(reg | BIT(data->offset), data->reg);
> +
> +       spin_unlock_irqrestore(data->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int sun4i_a10_display_status(struct reset_controller_dev *rcdev,
> +                                   unsigned long id)
> +{
> +       struct reset_data *data = rcdev_to_reset_data(rcdev);
> +
> +       return !(readl(data->reg) & BIT(data->offset));
> +}
> +
> +static const struct reset_control_ops sun4i_a10_display_reset_ops = {
> +       .assert         = sun4i_a10_display_assert,
> +       .deassert       = sun4i_a10_display_deassert,
> +       .status         = sun4i_a10_display_status,
> +};
> +
> +static int sun4i_a10_display_reset_xlate(struct reset_controller_dev *rcdev,
> +                                        const struct of_phandle_args *spec)
> +{
> +       /* We only have a single reset signal */
> +       return 0;
> +}
> +
> +static void __init sun4i_a10_display_init(struct device_node *node,
> +                                         struct sun4i_a10_display_clk_data *data)
> +{
> +       const char *parents[data->parents];
> +       const char *clk_name = node->name;
> +       struct reset_data *reset_data;
> +       struct clk_divider *div = NULL;
> +       struct clk_gate *gate;
> +       struct clk_mux *mux;
> +       void __iomem *reg;
> +       struct clk *clk;
> +       int i;
> +
> +       of_property_read_string(node, "clock-output-names", &clk_name);
> +
> +       reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> +       if (IS_ERR(reg)) {
> +               pr_err("%s: Could not map the clock registers\n", clk_name);
> +               return;
> +       }
> +
> +       of_clk_parent_fill(node, parents, data->parents);

What if someone uses an incorrect DT that has less parents described?

> +
> +       mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +       if (!mux)
> +               return;
> +
> +       mux->reg = reg;
> +       mux->shift = data->offset_mux;
> +       mux->mask = (1 << data->width_mux) - 1;
> +       mux->lock = &sun4i_a10_display_lock;
> +
> +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +       if (!gate)
> +               goto free_mux;
> +
> +       gate->reg = reg;
> +       gate->bit_idx = data->offset_en;
> +       gate->lock = &sun4i_a10_display_lock;
> +
> +       if (data->has_div) {
> +               div = kzalloc(sizeof(*div), GFP_KERNEL);
> +               if (!div)
> +                       goto free_gate;
> +
> +               div->reg = reg;
> +               div->shift = data->offset_div;
> +               div->width = data->width_div;
> +               div->lock = &sun4i_a10_display_lock;
> +       }
> +
> +       clk = clk_register_composite(NULL, clk_name,
> +                                    parents, data->parents,
> +                                    &mux->hw, &clk_mux_ops,
> +                                    data->has_div ? &div->hw : NULL,
> +                                    data->has_div ? &clk_divider_ops : NULL,
> +                                    &gate->hw, &clk_gate_ops,
> +                                    0);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s: Couldn't register the clock\n", clk_name);
> +               goto free_div;
> +       }
> +
> +       of_clk_add_provider(node, of_clk_src_simple_get, clk);

Check for errors?

> +
> +       if (!data->has_rst)
> +               return;
> +
> +       reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
> +       if (!reset_data)
> +               goto free_clk;
> +
> +       reset_data->reg = reg;
> +       reset_data->offset = data->offset_rst;
> +       reset_data->lock = &sun4i_a10_display_lock;
> +       reset_data->rcdev.nr_resets = 1;
> +       reset_data->rcdev.ops = &sun4i_a10_display_reset_ops;
> +       reset_data->rcdev.of_node = node;
> +       reset_data->rcdev.of_reset_n_cells = 0;
> +       reset_data->rcdev.of_xlate = &sun4i_a10_display_reset_xlate;
> +
> +       if (reset_controller_register(&reset_data->rcdev)) {
> +               pr_err("%s: Couldn't register the reset controller\n",
> +                      clk_name);
> +               goto free_reset;
> +       }
> +
> +       return;
> +
> +free_reset:
> +       kfree(reset_data);
> +free_clk:
> +       clk_unregister(clk);
> +free_div:
> +       if (data->has_div)
> +               kfree(div);
> +free_gate:
> +       kfree(gate);
> +free_mux:
> +       kfree(mux);

Cleanup after of_io_request_and_map()?

> +}
> +
> +static struct sun4i_a10_display_clk_data sun4i_a10_tcon_ch0_data = {
> +       .has_rst        = true,
> +       .parents        = 4,
> +       .offset_en      = 31,
> +       .offset_rst     = 29,

My datasheets (A10 v1.5, A20 v1.4), say bit 30 for LCD0 ch0 reset.

A13 and R8 manuals do not list this clock, but A10s does, which has
bit 30 for LCD reset and bit 29 for TV encoder reset.

I suggest changing has_rst to int, and for sun5i variant, have
has_rst = 2, reset_cells = 1, for 2 separate reset controls.
The tcon and tv encoder bits of the DTSI should be updated as
well.

I assume your tests work because U-boot already de-asserted the
resets (both if you're using composite output).

> +       .offset_mux     = 24,
> +       .width_mux      = 2,
> +};
> +
> +static void __init sun4i_a10_tcon_ch0_setup(struct device_node *node)
> +{
> +       sun4i_a10_display_init(node, &sun4i_a10_tcon_ch0_data);
> +}
> +CLK_OF_DECLARE(sun4i_a10_tcon_ch0, "allwinner,sun4i-a10-tcon-ch0-clk",
> +              sun4i_a10_tcon_ch0_setup);
> +
> +static struct sun4i_a10_display_clk_data sun4i_a10_display_data = {
> +       .has_div        = true,

Missing .has_rst = true.

> +       .parents        = 3,
> +       .offset_en      = 31,
> +       .offset_rst     = 30,
> +       .offset_mux     = 24,
> +       .offset_div     = 0,

This is already implied.

> +       .width_mux      = 2,
> +       .width_div      = 4,
> +};
> +
> +static void __init sun4i_a10_display_setup(struct device_node *node)
> +{
> +       sun4i_a10_display_init(node, &sun4i_a10_display_data);
> +}
> +CLK_OF_DECLARE(sun4i_a10_display, "allwinner,sun4i-a10-display-clk",
> +              sun4i_a10_display_setup);
> --
> 2.6.4

Thanks for working on this!

Regards
ChenYu

  parent reply	other threads:[~2016-01-16 15:30 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 15:24 [PATCH v2 00/26] drm: Add Allwinner A10 display engine support Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 01/26] reset: Move DT cell size check to the core Maxime Ripard
2016-01-15 15:50   ` Philipp Zabel
2016-01-14 15:24 ` [PATCH v2 02/26] reset: Make reset_control_ops const Maxime Ripard
2016-01-15 15:50   ` Philipp Zabel
2016-01-14 15:24 ` [PATCH v2 03/26] clk: Add regmap support Maxime Ripard
2016-01-28  7:56   ` Stephen Boyd
2016-01-14 15:24 ` [PATCH v2 04/26] clk: composite: Add unregister function Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 05/26] clk: sunxi: Add display and TCON0 clocks driver Maxime Ripard
2016-01-15  3:01   ` Rob Herring
2016-01-16 14:08   ` [linux-sunxi] " Priit Laes
2016-01-16 15:29   ` Chen-Yu Tsai [this message]
2016-02-03 20:18     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 06/26] clk: sunxi: Add PLL3 clock Maxime Ripard
2016-01-15  3:02   ` Rob Herring
2016-01-16 16:05   ` Chen-Yu Tsai
2016-02-03 20:27     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 07/26] clk: sunxi: Add TCON channel1 clock Maxime Ripard
2016-01-15  3:03   ` Rob Herring
2016-01-16 16:36   ` Chen-Yu Tsai
2016-02-03 20:29     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 08/26] clk: sun5i: add DRAM gates Maxime Ripard
2016-01-15  3:04   ` Rob Herring
2016-01-14 15:24 ` [PATCH v2 09/26] ARM: sun5i: dt: Add pll3 and pll7 clocks Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 10/26] ARM: sun5i: a13: Add display and TCON clocks Maxime Ripard
2016-01-16 17:06   ` Chen-Yu Tsai
2016-02-03 20:31     ` Maxime Ripard
2016-02-05  9:49       ` Chen-Yu Tsai
2016-01-14 15:24 ` [PATCH v2 11/26] ARM: sun5i: Add DRAM gates Maxime Ripard
2016-01-16 17:10   ` Chen-Yu Tsai
2016-02-03 20:36     ` Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 12/26] ARM: sun5i: Add TV encoder gate to the DTSI Maxime Ripard
2016-01-16 17:12   ` Chen-Yu Tsai
2016-01-14 15:24 ` [PATCH v2 13/26] drm/fb_cma_helper: Remove implicit call to disable_unused_functions Maxime Ripard
2016-01-14 23:13   ` Laurent Pinchart
2016-01-15 10:17     ` Daniel Vetter
2016-01-24 22:19       ` Laurent Pinchart
2016-01-25  7:29         ` Daniel Vetter
2016-01-25 19:02           ` Laurent Pinchart
2016-01-14 15:24 ` [PATCH v2 14/26] drm/modes: Rewrite the command line parser Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 15/26] drm/modes: Support modes names on the command line Maxime Ripard
2016-01-14 15:24 ` [PATCH v2 16/26] drm: Add Allwinner A10 Display Engine support Maxime Ripard
2016-01-16 15:11   ` [linux-sunxi] " Priit Laes
2016-01-17 12:58     ` Priit Laes
2016-01-19 15:38     ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 17/26] drm: sun4i: Add DT bindings documentation Maxime Ripard
2016-01-15  3:15   ` Rob Herring
2016-02-03 19:59     ` Maxime Ripard
2016-02-03 20:19       ` Rob Herring
2016-02-03 20:47         ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 18/26] drm: sun4i: Add RGB output Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 19/26] drm: sun4i: Add composite output Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 20/26] drm: sun4i: tv: Add PAL output standard Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 21/26] drm: sun4i: tv: Add NTSC " Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 22/26] ARM: sun5i: r8: Add display blocks to the DTSI Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 23/26] ARM: sun5i: chip: Enable the TV Encoder Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 24/26] devicetree: Add olimex vendor prefix Maxime Ripard
2016-01-15  3:15   ` Rob Herring
2016-01-15  6:41     ` Stefan Wahren
2016-01-15  8:05       ` Maxime Ripard
2016-01-14 15:25 ` [PATCH v2 25/26] drm/panel: simple: Add timings for the Olimex LCD-OLinuXino-4.3TS Maxime Ripard
2016-01-15  3:17   ` Rob Herring
2016-01-14 15:25 ` [PATCH v2 26/26] DO NOT MERGE: ARM: sun5i: chip: Enable the LCD panel Maxime Ripard
2016-02-20 13:45 ` [linux-sunxi] [PATCH v2 00/26] drm: Add Allwinner A10 display engine support Priit Laes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAGb2v65=EVTQo8KFMECVR4=94i0rt0V1H07hWKyXShSZxGYKEQ@mail.gmail.com' \
    --to=wens@csie.org \
    --cc=airlied@linux.ie \
    --cc=alex@nextthing.co \
    --cc=boris.brezillon@free-electrons.com \
    --cc=daniel@ffwll.ch \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=maxime.ripard@free-electrons.com \
    --cc=mturquette@baylibre.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robdclark@gmail.com \
    --cc=sboyd@codeaurora.org \
    --cc=thierry.reding@gmail.com \
    --cc=thomas.petazzoni@free-electrons.com \
    --cc=wynter@nextthing.co \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).