From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0FD581C6AF for ; Thu, 4 Apr 2024 16:59:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712249998; cv=none; b=fjCxTzeZxTCB7OOjbxPOVSb85DZJPrYVuwB7NumF8U1J3q5e/9NNGXhx6aP2RxtLSfeuW7s6dj+XsXK67SnSs2MOQSeO379UOsEpn8/aMzRXGU2fhV/CXnDLYLrvOfZNPf0A+q6bwoAOVAvnDw5LCxy7bbCcERs+gvN3eMO3HlY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712249998; c=relaxed/simple; bh=S1uejGD3rw4ojb+Bm8r9lKR77Mau0REFWjYZV2BkZKQ=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=GiI+dHCQluRqhc8pY7KDCoutoO9gPApupBjUuYU8F4F7+s4ILwVgw7wrY6E8AxlzjQplAGbcmti3Pu1OYMaQO/MBKEqeJC2pz+WvTbrcWli43MOYHbiMIVH1KZFbDK8hPvqEYaIvnDRBztvJt3RYX9lwlPpF/36/PslS557RYaw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=zbeDyalJ; arc=none smtp.client-ip=209.85.221.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="zbeDyalJ" Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-3436b096690so1837272f8f.1 for ; Thu, 04 Apr 2024 09:59:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1712249993; x=1712854793; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:organization:autocrypt :content-language:references:cc:to:subject:reply-to:from:user-agent :mime-version:date:message-id:from:to:cc:subject:date:message-id :reply-to; bh=l3Sy/O7JrPcSJpu77aPp9TPQX84YpKNhe7+xiQxZncY=; b=zbeDyalJ38fiUeNSVv+F9JdeQdjIgfodtep9dwii/GtQRaaWHiomCqgO8G78m96hH6 vL7CZuFKBYHsXvnHOKTuVMGAiVwVB5mvsg+5a7GzAomn7iqo2sKHRqsrlu5z19qOf6bp VkkPc3hEKtDkt/DBnQ9TbHy7Fv8XMaybvIed6hDq4KKy6ZIirfrTFzqeT6fit6W4Bwbz q4umNwmLOhqKLyFrFOBKcuseBejYSjkV58OgPQGnwZVAGsEXxdtw9EHq/E+5IJcTX8NH HyD28eFts0AFlvMjtTAX8kxbNdmD4d0q36MdrJ62vjameuAUbNDe0ppFtQm/fFOwrFsU /T5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712249993; x=1712854793; h=content-transfer-encoding:in-reply-to:organization:autocrypt :content-language:references:cc:to:subject:reply-to:from:user-agent :mime-version:date:message-id:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=l3Sy/O7JrPcSJpu77aPp9TPQX84YpKNhe7+xiQxZncY=; b=X5sV/Eo3lNpp13SjBJHysE+/F8C3vWITDxZeIEeskpLPdI4RxPnkRHJIMR91xna9Ja 8xb2MTsiRX8XUHoc6wKHeCOrS55ZzUQ249gwGrjqsEhWIq0mzcTB1HxRvfE+raP8V0it gklOFM4dX01MNivdqjqmfS99XOfoAJzSdMx9slfKMfvM4LY3zNa+7v3JLjzX/w/kT2Bk cNHPWdUSjF4BKzgIjxIzMz9n0xAXNAex2JkQBqQUZo1AVcCSw2VITZQ3Fysd3pzlYmrl sAGpeoxzJAVx0piPWig56L+QoncbkVj5xfDlfZsSoPabKMxFWLi+29MlF8ooA142MLvZ mKFg== X-Forwarded-Encrypted: i=1; AJvYcCUe/SDbrSjExEZ3bdHJbzfvYv261xDmxnfiLnoCn459qAfa7lKHKf0XhpfCEnaw/93iJMWCkhCimkkUd47N3IsJNX1HT35eoy0k X-Gm-Message-State: AOJu0YwgWDCOjMakSgCIn8QfwSBP7BVz/OEh3S/RPdYwWm/8OouyraF5 N9JmtpcFr7ZpxqpmstQ53XJbquCqNEEZR2fRHdsrajgKziCL8N5J8bT2kySRtuQ= X-Google-Smtp-Source: AGHT+IFjXaNrqfypx/RIQ+/6bcfFkAfBL80K/QjtfVHCoqExHR3ODQL4yTaaXd1xO2D4BDgljPA4/A== X-Received: by 2002:adf:ab1e:0:b0:343:6b42:b3bb with SMTP id q30-20020adfab1e000000b003436b42b3bbmr35091wrc.31.1712249993185; Thu, 04 Apr 2024 09:59:53 -0700 (PDT) Received: from ?IPV6:2a01:e0a:982:cbb0:5910:95db:d5a3:46d6? ([2a01:e0a:982:cbb0:5910:95db:d5a3:46d6]) by smtp.gmail.com with ESMTPSA id ce8-20020a5d5e08000000b003433e4c6d43sm16950894wrb.32.2024.04.04.09.59.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 04 Apr 2024 09:59:52 -0700 (PDT) Message-ID: <2cf79f07-0ae1-4267-ac08-fe40366d87d4@linaro.org> Date: Thu, 4 Apr 2024 18:59:51 +0200 Precedence: bulk X-Mailing-List: linux-clk@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Neil Armstrong Reply-To: neil.armstrong@linaro.org Subject: Re: [PATCH v12 2/7] clk: meson: add vclk driver To: Jerome Brunet Cc: Rob Herring , Krzysztof Kozlowski , Conor Dooley , Martin Blumenstingl , Kevin Hilman , Michael Turquette , Stephen Boyd , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Daniel Vetter , Jagan Teki , Nicolas Belin , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, dri-devel@lists.freedesktop.org References: <20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-0-99ecdfdc87fc@linaro.org> <20240403-amlogic-v6-4-upstream-dsi-ccf-vim3-v12-2-99ecdfdc87fc@linaro.org> <1jmsq9pmgd.fsf@starbuckisacylon.baylibre.com> Content-Language: en-US, fr Autocrypt: addr=neil.armstrong@linaro.org; keydata= xsBNBE1ZBs8BCAD78xVLsXPwV/2qQx2FaO/7mhWL0Qodw8UcQJnkrWmgTFRobtTWxuRx8WWP GTjuhvbleoQ5Cxjr+v+1ARGCH46MxFP5DwauzPekwJUD5QKZlaw/bURTLmS2id5wWi3lqVH4 BVF2WzvGyyeV1o4RTCYDnZ9VLLylJ9bneEaIs/7cjCEbipGGFlfIML3sfqnIvMAxIMZrvcl9 qPV2k+KQ7q+aXavU5W+yLNn7QtXUB530Zlk/d2ETgzQ5FLYYnUDAaRl+8JUTjc0CNOTpCeik 80TZcE6f8M76Xa6yU8VcNko94Ck7iB4vj70q76P/J7kt98hklrr85/3NU3oti3nrIHmHABEB AAHNKk5laWwgQXJtc3Ryb25nIDxuZWlsLmFybXN0cm9uZ0BsaW5hcm8ub3JnPsLAkQQTAQoA OwIbIwULCQgHAwUVCgkICwUWAgMBAAIeAQIXgBYhBInsPQWERiF0UPIoSBaat7Gkz/iuBQJk Q5wSAhkBAAoJEBaat7Gkz/iuyhMIANiD94qDtUTJRfEW6GwXmtKWwl/mvqQtaTtZID2dos04 YqBbshiJbejgVJjy+HODcNUIKBB3PSLaln4ltdsV73SBcwUNdzebfKspAQunCM22Mn6FBIxQ GizsMLcP/0FX4en9NaKGfK6ZdKK6kN1GR9YffMJd2P08EO8mHowmSRe/ExAODhAs9W7XXExw UNCY4pVJyRPpEhv373vvff60bHxc1k/FF9WaPscMt7hlkbFLUs85kHtQAmr8pV5Hy9ezsSRa GzJmiVclkPc2BY592IGBXRDQ38urXeM4nfhhvqA50b/nAEXc6FzqgXqDkEIwR66/Gbp0t3+r yQzpKRyQif3OwE0ETVkGzwEIALyKDN/OGURaHBVzwjgYq+ZtifvekdrSNl8TIDH8g1xicBYp QTbPn6bbSZbdvfeQPNCcD4/EhXZuhQXMcoJsQQQnO4vwVULmPGgtGf8PVc7dxKOeta+qUh6+ SRh3vIcAUFHDT3f/Zdspz+e2E0hPV2hiSvICLk11qO6cyJE13zeNFoeY3ggrKY+IzbFomIZY 4yG6xI99NIPEVE9lNBXBKIlewIyVlkOaYvJWSV+p5gdJXOvScNN1epm5YHmf9aE2ZjnqZGoM Mtsyw18YoX9BqMFInxqYQQ3j/HpVgTSvmo5ea5qQDDUaCsaTf8UeDcwYOtgI8iL4oHcsGtUX oUk33HEAEQEAAcLAXwQYAQIACQUCTVkGzwIbDAAKCRAWmrexpM/4rrXiB/sGbkQ6itMrAIfn M7IbRuiSZS1unlySUVYu3SD6YBYnNi3G5EpbwfBNuT3H8//rVvtOFK4OD8cRYkxXRQmTvqa3 3eDIHu/zr1HMKErm+2SD6PO9umRef8V82o2oaCLvf4WeIssFjwB0b6a12opuRP7yo3E3gTCS KmbUuLv1CtxKQF+fUV1cVaTPMyT25Od+RC1K+iOR0F54oUJvJeq7fUzbn/KdlhA8XPGzwGRy 4zcsPWvwnXgfe5tk680fEKZVwOZKIEuJC3v+/yZpQzDvGYJvbyix0lHnrCzq43WefRHI5XTT QbM0WUIBIcGmq38+OgUsMYu4NzLu7uZFAcmp6h8g Organization: Linaro In-Reply-To: <1jmsq9pmgd.fsf@starbuckisacylon.baylibre.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 04/04/2024 10:13, Jerome Brunet wrote: > > On Wed 03 Apr 2024 at 09:46, Neil Armstrong wrote: > >> The VCLK and VCLK_DIV clocks have supplementary bits. >> >> The VCLK gate has a "SOFT RESET" bit to toggle after the whole >> VCLK sub-tree rate has been set, this is implemented in >> the gate enable callback. >> >> The VCLK_DIV clocks as enable and reset bits used to disable >> and reset the divider, associated with CLK_SET_RATE_GATE it ensures >> the rate is set while the divider is disabled and in reset mode. >> >> The VCLK_DIV enable bit isn't implemented as a gate since it's part >> of the divider logic and vendor does this exact sequence to ensure >> the divider is correctly set. > > The checkpatch warning is still there. Is it a choice or a mistake ? > > Documentation says "GPL v2" exists for historic reason which seems to > hint "GPL" is preferred, and I suppose this is why checkpatch warns for > it. Well I didn't see this warning, this is what I fixed: $ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c CHECK: Alignment should match open parenthesis #63: FILE: drivers/clk/meson/vclk.c:63: +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw, + unsigned long prate) CHECK: Alignment should match open parenthesis #73: FILE: drivers/clk/meson/vclk.c:73: +static int meson_vclk_div_determine_rate(struct clk_hw *hw, + struct clk_rate_request *req) CHECK: Alignment should match open parenthesis #83: FILE: drivers/clk/meson/vclk.c:83: +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) It seems that checking a commit triggers an extra check.... $ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3 WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #58: new file mode 100644 WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity") #203: FILE: drivers/clk/meson/vclk.c:141: +MODULE_LICENSE("GPL v2"); Neil > >> >> Signed-off-by: Neil Armstrong >> --- >> drivers/clk/meson/Kconfig | 4 ++ >> drivers/clk/meson/Makefile | 1 + >> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++ >> drivers/clk/meson/vclk.h | 51 ++++++++++++++++ >> 4 files changed, 197 insertions(+) >> >> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig >> index 29ffd14d267b..8a9823789fa3 100644 >> --- a/drivers/clk/meson/Kconfig >> +++ b/drivers/clk/meson/Kconfig >> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV >> tristate >> select COMMON_CLK_MESON_REGMAP >> >> +config COMMON_CLK_MESON_VCLK >> + tristate >> + select COMMON_CLK_MESON_REGMAP >> + >> config COMMON_CLK_MESON_CLKC_UTILS >> tristate >> >> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile >> index 9ee4b954c896..9ba43fe7a07a 100644 >> --- a/drivers/clk/meson/Makefile >> +++ b/drivers/clk/meson/Makefile >> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o >> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o >> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o >> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o >> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o >> >> # Amlogic Clock controllers >> >> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c >> new file mode 100644 >> index 000000000000..45dc216941ea >> --- /dev/null >> +++ b/drivers/clk/meson/vclk.c >> @@ -0,0 +1,141 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2024 Neil Armstrong >> + */ >> + >> +#include >> +#include "vclk.h" >> + >> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */ >> + >> +static inline struct meson_vclk_gate_data * >> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk) >> +{ >> + return (struct meson_vclk_gate_data *)clk->data; >> +} >> + >> +static int meson_vclk_gate_enable(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk); >> + >> + meson_parm_write(clk->map, &vclk->enable, 1); >> + >> + /* Do a reset pulse */ >> + meson_parm_write(clk->map, &vclk->reset, 1); >> + meson_parm_write(clk->map, &vclk->reset, 0); >> + >> + return 0; >> +} >> + >> +static void meson_vclk_gate_disable(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk); >> + >> + meson_parm_write(clk->map, &vclk->enable, 0); >> +} >> + >> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk); >> + >> + return meson_parm_read(clk->map, &vclk->enable); >> +} >> + >> +const struct clk_ops meson_vclk_gate_ops = { >> + .enable = meson_vclk_gate_enable, >> + .disable = meson_vclk_gate_disable, >> + .is_enabled = meson_vclk_gate_is_enabled, >> +}; >> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops); >> + >> +/* The VCLK Divider has supplementary reset & enable bits */ >> + >> +static inline struct meson_vclk_div_data * >> +clk_get_meson_vclk_div_data(struct clk_regmap *clk) >> +{ >> + return (struct meson_vclk_div_data *)clk->data; >> +} >> + >> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw, >> + unsigned long prate) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + >> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div), >> + vclk->table, vclk->flags, vclk->div.width); >> +} >> + >> +static int meson_vclk_div_determine_rate(struct clk_hw *hw, >> + struct clk_rate_request *req) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + >> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width, >> + vclk->flags); >> +} >> + >> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate, >> + unsigned long parent_rate) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + int ret; >> + >> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width, >> + vclk->flags); >> + if (ret < 0) >> + return ret; >> + >> + meson_parm_write(clk->map, &vclk->div, ret); >> + >> + return 0; >> +}; >> + >> +static int meson_vclk_div_enable(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + >> + /* Unreset the divider when ungating */ >> + meson_parm_write(clk->map, &vclk->reset, 0); >> + meson_parm_write(clk->map, &vclk->enable, 1); >> + >> + return 0; >> +} >> + >> +static void meson_vclk_div_disable(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + >> + /* Reset the divider when gating */ >> + meson_parm_write(clk->map, &vclk->enable, 0); >> + meson_parm_write(clk->map, &vclk->reset, 1); >> +} >> + >> +static int meson_vclk_div_is_enabled(struct clk_hw *hw) >> +{ >> + struct clk_regmap *clk = to_clk_regmap(hw); >> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk); >> + >> + return meson_parm_read(clk->map, &vclk->enable); >> +} >> + >> +const struct clk_ops meson_vclk_div_ops = { >> + .recalc_rate = meson_vclk_div_recalc_rate, >> + .determine_rate = meson_vclk_div_determine_rate, >> + .set_rate = meson_vclk_div_set_rate, >> + .enable = meson_vclk_div_enable, >> + .disable = meson_vclk_div_disable, >> + .is_enabled = meson_vclk_div_is_enabled, >> +}; >> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops); >> + >> +MODULE_DESCRIPTION("Amlogic vclk clock driver"); >> +MODULE_AUTHOR("Neil Armstrong "); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h >> new file mode 100644 >> index 000000000000..20b0b181db09 >> --- /dev/null >> +++ b/drivers/clk/meson/vclk.h >> @@ -0,0 +1,51 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Copyright (c) 2024 Neil Armstrong >> + */ >> + >> +#ifndef __VCLK_H >> +#define __VCLK_H >> + >> +#include "clk-regmap.h" >> +#include "parm.h" >> + >> +/** >> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data >> + * >> + * @enable: vclk enable field >> + * @reset: vclk reset field >> + * @flags: hardware-specific flags >> + * >> + * Flags: >> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored >> + */ >> +struct meson_vclk_gate_data { >> + struct parm enable; >> + struct parm reset; >> + u8 flags; >> +}; >> + >> +extern const struct clk_ops meson_vclk_gate_ops; >> + >> +/** >> + * struct meson_vclk_div_data - vclk_div regmap back specific data >> + * >> + * @div: divider field >> + * @enable: vclk divider enable field >> + * @reset: vclk divider reset field >> + * @table: array of value/divider pairs, last entry should have div = 0 >> + * >> + * Flags: >> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored >> + */ >> +struct meson_vclk_div_data { >> + struct parm div; >> + struct parm enable; >> + struct parm reset; >> + const struct clk_div_table *table; >> + u8 flags; >> +}; >> + >> +extern const struct clk_ops meson_vclk_div_ops; >> + >> +#endif /* __VCLK_H */ > >