* Re: [PATCH 2/2] arm64: dts: ti: k3-am642-phyboard-electra-rdk: Increase CAN max bitrate
From: Nishanth Menon @ 2024-04-02 16:14 UTC (permalink / raw)
To: Nathan Morrisson
Cc: vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt,
linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
In-Reply-To: <20240402161203.q34gyqfaoewvjbky@unburned>
On 11:12-20240402, Nishanth Menon wrote:
> On 09:08-20240402, Nathan Morrisson wrote:
> > The phyBOARD-Electra has two TCAN1044VDD CAN transceivers which
> > support CAN FD at 8 Mbps.
> >
> > Increase the maximum bitrate to 8 Mbps.
> >
> > Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
> > ---
> > arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > index 8237b8c815b8..522699ec65e8 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> > @@ -42,7 +42,7 @@ can_tc1: can-phy0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&can_tc1_pins_default>;
> > #phy-cells = <0>;
> > - max-bitrate = <5000000>;
> > + max-bitrate = <8000000>;
> > standby-gpios = <&main_gpio0 32 GPIO_ACTIVE_HIGH>;
> > };
> >
> > @@ -51,7 +51,7 @@ can_tc2: can-phy1 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&can_tc2_pins_default>;
> > #phy-cells = <0>;
> > - max-bitrate = <5000000>;
> > + max-bitrate = <8000000>;
> > standby-gpios = <&main_gpio0 35 GPIO_ACTIVE_HIGH>;
> > };
> >
> > --
> > 2.25.1
> >
>
> This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
>
Woops.. wrong mail thread. :( Apologies on the spam.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Nishanth Menon @ 2024-04-02 16:13 UTC (permalink / raw)
To: Michael Walle
Cc: Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <20240402151802.3803708-1-mwalle@kernel.org>
On 17:18-20240402, Michael Walle wrote:
> Device tree best practice is to disable any external interface in the
> dtsi and just enable them if needed in the device tree. Thus, disable
> both ethernet ports by default and just enable the one used by the EVM
> in its device tree.
>
> There is no functional change.
>
> Signed-off-by: Michael Walle <mwalle@kernel.org>
> ---
> This should also be true for all the other SoCs. But I don't wanted to
> touch all the (older) device trees. j722s is pretty new, so there we
> should get it right.
> ---
> arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
> arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> index d045dc7dde0c..afe7f68e6a4b 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
> @@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
> };
>
> &cpsw_port1 {
> + status = "okay";
> phy-mode = "rgmii-rxid";
> phy-handle = <&cpsw3g_phy0>;
> };
>
> -&cpsw_port2 {
> - status = "disabled";
> -};
> -
> &main_gpio1 {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> index c75744edb143..d0451e6e7496 100644
> --- a/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
> @@ -83,6 +83,14 @@ &inta_main_dmss {
> ti,interrupt-ranges = <7 71 21>;
> };
>
> +&cpsw_port1 {
> + status = "disabled";
> +};
> +
> +&cpsw_port2 {
> + status = "disabled";
> +};
> +
> &oc_sram {
> reg = <0x00 0x70000000 0x00 0x40000>;
> ranges = <0x00 0x00 0x70000000 0x40000>;
> --
> 2.39.2
>
Meant to respond to this thread:
This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH v6 1/4] dt-bindings: clock: add i.MX95 clock header
From: Rob Herring @ 2024-04-02 16:13 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: linux-clk, Krzysztof Kozlowski, Shawn Guo, Sascha Hauer, Peng Fan,
Abel Vesa, Pengutronix Kernel Team, devicetree, linux-arm-kernel,
Michael Turquette, Stephen Boyd, Fabio Estevam, imx, linux-kernel,
Conor Dooley
In-Reply-To: <20240401-imx95-blk-ctl-v6-1-84d4eca1e759@nxp.com>
On Mon, 01 Apr 2024 21:28:15 +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add clock header for i.MX95 BLK CTL modules
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> include/dt-bindings/clock/nxp,imx95-clock.h | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] ASoC: dt-bindings: fsl-asoc-card: convert to YAML
From: Rob Herring @ 2024-04-02 16:12 UTC (permalink / raw)
To: Shengjiu Wang
Cc: festevam, shengjiu.wang, shawnguo, broonie, linux-kernel, robh+dt,
linux-arm-kernel, krzysztof.kozlowski+dt, conor+dt, imx, s.hauer,
lgirdwood, linux-sound, kernel, devicetree
In-Reply-To: <1711976056-19884-3-git-send-email-shengjiu.wang@nxp.com>
On Mon, 01 Apr 2024 20:54:16 +0800, Shengjiu Wang wrote:
> Convert the fsl-asoc-card binding to YAML.
>
> When testing dtbs_check, found below compatible strings
> are not listed in document:
>
> fsl,imx-sgtl5000
> fsl,imx53-cpuvo-sgtl5000
> fsl,imx51-babbage-sgtl5000
> fsl,imx53-m53evk-sgtl5000
> fsl,imx53-qsb-sgtl5000
> fsl,imx53-voipac-sgtl5000
> fsl,imx6-armadeus-sgtl5000
> fsl,imx6-rex-sgtl5000
> fsl,imx6-sabreauto-cs42888
> fsl,imx6-wandboard-sgtl5000
> fsl,imx6dl-nit6xlite-sgtl5000
> fsl,imx6q-ba16-sgtl5000
> fsl,imx6q-nitrogen6_max-sgtl5000
> fsl,imx6q-nitrogen6_som2-sgtl5000
> fsl,imx6q-nitrogen6x-sgtl5000
> fsl,imx6q-sabrelite-sgtl5000
> fsl,imx6q-sabresd-wm8962
> fsl,imx6q-udoo-ac97
> fsl,imx6q-ventana-sgtl5000
> fsl,imx6sl-evk-wm8962
> fsl,imx6sx-sdb-mqs
> fsl,imx6sx-sdb-wm8962
> fsl,imx7d-evk-wm8960
> karo,tx53-audio-sgtl5000
> tq,imx53-mba53-sgtl5000
>
> So add them in yaml file to pass the test.
>
> Also correct the 'dai-format' to 'format' in document.
>
> For 'audio-routing', the items are not listed. Because
> this fsl-asoc-card is generic driver, which supports several
> codecs, if list all the items, there will be a long list.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
> .../bindings/sound/fsl-asoc-card.txt | 117 -----------
> .../bindings/sound/fsl-asoc-card.yaml | 195 ++++++++++++++++++
> 2 files changed, 195 insertions(+), 117 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/sound/fsl-asoc-card.txt
> create mode 100644 Documentation/devicetree/bindings/sound/fsl-asoc-card.yaml
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: ti: k3-am642-phyboard-electra-rdk: Increase CAN max bitrate
From: Nishanth Menon @ 2024-04-02 16:12 UTC (permalink / raw)
To: Nathan Morrisson
Cc: vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt,
linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
In-Reply-To: <20240402160825.1516036-3-nmorrisson@phytec.com>
On 09:08-20240402, Nathan Morrisson wrote:
> The phyBOARD-Electra has two TCAN1044VDD CAN transceivers which
> support CAN FD at 8 Mbps.
>
> Increase the maximum bitrate to 8 Mbps.
>
> Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
> ---
> arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> index 8237b8c815b8..522699ec65e8 100644
> --- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
> @@ -42,7 +42,7 @@ can_tc1: can-phy0 {
> pinctrl-names = "default";
> pinctrl-0 = <&can_tc1_pins_default>;
> #phy-cells = <0>;
> - max-bitrate = <5000000>;
> + max-bitrate = <8000000>;
> standby-gpios = <&main_gpio0 32 GPIO_ACTIVE_HIGH>;
> };
>
> @@ -51,7 +51,7 @@ can_tc2: can-phy1 {
> pinctrl-names = "default";
> pinctrl-0 = <&can_tc2_pins_default>;
> #phy-cells = <0>;
> - max-bitrate = <5000000>;
> + max-bitrate = <8000000>;
> standby-gpios = <&main_gpio0 35 GPIO_ACTIVE_HIGH>;
> };
>
> --
> 2.25.1
>
This is better at arch/arm64/boot/dts/ti/k3-am62p-main.dtsi.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
^ permalink raw reply
* Re: [PATCH v1 6/6] clk: meson: a1: add Amlogic A1 CPU clock controller driver
From: Dmitry Rokosov @ 2024-04-02 16:11 UTC (permalink / raw)
To: Jerome Brunet
Cc: neil.armstrong, mturquette, sboyd, robh+dt,
krzysztof.kozlowski+dt, khilman, martin.blumenstingl, kernel,
rockosov, linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <1jr0fnj11b.fsf@starbuckisacylon.baylibre.com>
On Tue, Apr 02, 2024 at 04:11:12PM +0200, Jerome Brunet wrote:
>
> On Tue 02 Apr 2024 at 14:05, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
>
> > Hello Jerome,
> >
> > On Tue, Apr 02, 2024 at 11:35:49AM +0200, Jerome Brunet wrote:
> >>
> >> On Fri 29 Mar 2024 at 23:58, Dmitry Rokosov <ddrokosov@salutedevices.com> wrote:
> >>
> >> > The CPU clock controller plays a general role in the Amlogic A1 SoC
> >> > family by generating CPU clocks. As an APB slave module, it offers the
> >> > capability to inherit the CPU clock from two sources: the internal fixed
> >> > clock known as 'cpu fixed clock' and the external input provided by the
> >> > A1 PLL clock controller, referred to as 'syspll'.
> >> >
> >> > It is important for the driver to handle cpu_clk rate switching
> >> > effectively by transitioning to the CPU fixed clock to avoid any
> >> > potential execution freezes.
> >> >
> >> > Signed-off-by: Dmitry Rokosov <ddrokosov@salutedevices.com>
> >> > ---
> >> > drivers/clk/meson/Kconfig | 10 ++
> >> > drivers/clk/meson/Makefile | 1 +
> >> > drivers/clk/meson/a1-cpu.c | 324 +++++++++++++++++++++++++++++++++++++
> >> > drivers/clk/meson/a1-cpu.h | 16 ++
> >> > 4 files changed, 351 insertions(+)
> >> > create mode 100644 drivers/clk/meson/a1-cpu.c
> >> > create mode 100644 drivers/clk/meson/a1-cpu.h
> >> >
> >> > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> >> > index 80c4a18c83d2..148d4495eee3 100644
> >> > --- a/drivers/clk/meson/Kconfig
> >> > +++ b/drivers/clk/meson/Kconfig
> >> > @@ -111,6 +111,16 @@ config COMMON_CLK_AXG_AUDIO
> >> > Support for the audio clock controller on AmLogic A113D devices,
> >> > aka axg, Say Y if you want audio subsystem to work.
> >> >
> >> > +config COMMON_CLK_A1_CPU
> >> > + tristate "Amlogic A1 SoC CPU controller support"
> >> > + depends on ARM64
> >> > + select COMMON_CLK_MESON_REGMAP
> >> > + select COMMON_CLK_MESON_CLKC_UTILS
> >> > + help
> >> > + Support for the CPU clock controller on Amlogic A113L based
> >> > + device, A1 SoC Family. Say Y if you want A1 CPU clock controller
> >> > + to work.
> >> > +
> >> > config COMMON_CLK_A1_PLL
> >> > tristate "Amlogic A1 SoC PLL controller support"
> >> > depends on ARM64
> >> > diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> >> > index 4968fc7ad555..2a06eb0303d6 100644
> >> > --- a/drivers/clk/meson/Makefile
> >> > +++ b/drivers/clk/meson/Makefile
> >> > @@ -18,6 +18,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_AUDIO_RSTC) += meson-audio-rstc.o
> >> >
> >> > obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
> >> > obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> >> > +obj-$(CONFIG_COMMON_CLK_A1_CPU) += a1-cpu.o
> >> > obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
> >> > obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
> >> > obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
> >> > diff --git a/drivers/clk/meson/a1-cpu.c b/drivers/clk/meson/a1-cpu.c
> >> > new file mode 100644
> >> > index 000000000000..5f5d8ae112e5
> >> > --- /dev/null
> >> > +++ b/drivers/clk/meson/a1-cpu.c
> >> > @@ -0,0 +1,324 @@
> >> > +// SPDX-License-Identifier: GPL-2.0+
> >> > +/*
> >> > + * Amlogic A1 SoC family CPU Clock Controller driver.
> >> > + *
> >> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> >> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
> >> > + */
> >> > +
> >> > +#include <linux/clk.h>
> >> > +#include <linux/clk-provider.h>
> >> > +#include <linux/mod_devicetable.h>
> >> > +#include <linux/platform_device.h>
> >> > +#include "a1-cpu.h"
> >> > +#include "clk-regmap.h"
> >> > +#include "meson-clkc-utils.h"
> >> > +
> >> > +#include <dt-bindings/clock/amlogic,a1-cpu-clkc.h>
> >> > +
> >> > +static u32 cpu_fsource_sel_table[] = { 0, 1, 2 };
> >> > +static const struct clk_parent_data cpu_fsource_sel_parents[] = {
> >> > + { .fw_name = "xtal" },
> >> > + { .fw_name = "fclk_div2" },
> >> > + { .fw_name = "fclk_div3" },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsource_sel0 = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x3,
> >> > + .shift = 0,
> >> > + .table = cpu_fsource_sel_table,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsource_sel0",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_data = cpu_fsource_sel_parents,
> >> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsource_div0 = {
> >> > + .data = &(struct clk_regmap_div_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .shift = 4,
> >> > + .width = 6,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsource_div0",
> >> > + .ops = &clk_regmap_divider_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsource_sel0.hw
> >> > + },
> >> > + .num_parents = 1,
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsel0 = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x1,
> >> > + .shift = 2,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsel0",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsource_sel0.hw,
> >> > + &cpu_fsource_div0.hw,
> >> > + },
> >> > + .num_parents = 2,
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsource_sel1 = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x3,
> >> > + .shift = 16,
> >> > + .table = cpu_fsource_sel_table,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsource_sel1",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_data = cpu_fsource_sel_parents,
> >> > + .num_parents = ARRAY_SIZE(cpu_fsource_sel_parents),
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsource_div1 = {
> >> > + .data = &(struct clk_regmap_div_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .shift = 20,
> >> > + .width = 6,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsource_div1",
> >> > + .ops = &clk_regmap_divider_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsource_sel1.hw
> >> > + },
> >> > + .num_parents = 1,
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fsel1 = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x1,
> >> > + .shift = 18,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fsel1",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsource_sel1.hw,
> >> > + &cpu_fsource_div1.hw,
> >> > + },
> >> > + .num_parents = 2,
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_fclk = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x1,
> >> > + .shift = 10,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_fclk",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_hws = (const struct clk_hw *[]) {
> >> > + &cpu_fsel0.hw,
> >> > + &cpu_fsel1.hw,
> >> > + },
> >> > + .num_parents = 2,
> >> > + .flags = CLK_SET_RATE_PARENT,
> >> > + },
> >> > +};
> >> > +
> >> > +static struct clk_regmap cpu_clk = {
> >> > + .data = &(struct clk_regmap_mux_data) {
> >> > + .offset = CPUCTRL_CLK_CTRL0,
> >> > + .mask = 0x1,
> >> > + .shift = 11,
> >> > + },
> >> > + .hw.init = &(struct clk_init_data) {
> >> > + .name = "cpu_clk",
> >> > + .ops = &clk_regmap_mux_ops,
> >> > + .parent_data = (const struct clk_parent_data []) {
> >> > + { .hw = &cpu_fclk.hw },
> >> > + { .fw_name = "sys_pll", },
> >> > + },
> >> > + .num_parents = 2,
> >> > + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> >> > + },
> >> > +};
> >> > +
> >> > +/* Array of all clocks registered by this provider */
> >> > +static struct clk_hw *a1_cpu_hw_clks[] = {
> >> > + [CLKID_CPU_FSOURCE_SEL0] = &cpu_fsource_sel0.hw,
> >> > + [CLKID_CPU_FSOURCE_DIV0] = &cpu_fsource_div0.hw,
> >> > + [CLKID_CPU_FSEL0] = &cpu_fsel0.hw,
> >> > + [CLKID_CPU_FSOURCE_SEL1] = &cpu_fsource_sel1.hw,
> >> > + [CLKID_CPU_FSOURCE_DIV1] = &cpu_fsource_div1.hw,
> >> > + [CLKID_CPU_FSEL1] = &cpu_fsel1.hw,
> >> > + [CLKID_CPU_FCLK] = &cpu_fclk.hw,
> >> > + [CLKID_CPU_CLK] = &cpu_clk.hw,
> >> > +};
> >> > +
> >> > +static struct clk_regmap *const a1_cpu_regmaps[] = {
> >> > + &cpu_fsource_sel0,
> >> > + &cpu_fsource_div0,
> >> > + &cpu_fsel0,
> >> > + &cpu_fsource_sel1,
> >> > + &cpu_fsource_div1,
> >> > + &cpu_fsel1,
> >> > + &cpu_fclk,
> >> > + &cpu_clk,
> >> > +};
> >> > +
> >> > +static struct regmap_config a1_cpu_regmap_cfg = {
> >> > + .reg_bits = 32,
> >> > + .val_bits = 32,
> >> > + .reg_stride = 4,
> >> > + .max_register = CPUCTRL_CLK_CTRL1,
> >> > +};
> >> > +
> >> > +static struct meson_clk_hw_data a1_cpu_clks = {
> >> > + .hws = a1_cpu_hw_clks,
> >> > + .num = ARRAY_SIZE(a1_cpu_hw_clks),
> >> > +};
> >> > +
> >> > +struct a1_cpu_clk_nb_data {
> >> > + const struct clk_ops *mux_ops;
> >>
> >> That's fishy ...
> >>
> >> > + struct clk_hw *cpu_clk;
> >> > + struct notifier_block nb;
> >> > + u8 parent;
> >> > +};
> >> > +
> >> > +#define MESON_A1_CPU_CLK_GET_PARENT(nbd) \
> >> > + ((nbd)->mux_ops->get_parent((nbd)->cpu_clk))
> >> > +#define MESON_A1_CPU_CLK_SET_PARENT(nbd, index) \
> >> > + ((nbd)->mux_ops->set_parent((nbd)->cpu_clk, index))
> >>
> >> ... Directly going for the mux ops ??!?? No way !
> >>
> >> We have a framework to handle the clocks, the whole point is to use it,
> >> not bypass it !
> >>
> >
> > I suppose you understand my approach, which is quite similar to what is
> > happening in the Mediatek driver:
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/clk/mediatek/clk-mux.c#L295
> >
> > Initially, I attempted to set the parent using the clk_set_parent() API.
> > However, I encountered a problem with recursive calling of the
> > notifier_block. This issue arises because the parent triggers
> > notifications for its children, leading to repeated calls to the
> > notifier_block.
> >
> > I find it puzzling why I cannot call an internal function or callback
> > within the internal driver context. After all, the notifier block is
> > just a part of the set_rate() flow. From a global Clock Control
> > Framework perspective, the context should not change.
>
> I don't care what MTK is doing TBH. Forcefully calling ops on a clock,
> hoping they are going to match with the clock type is wrong.
>
> There is a clk_hw API. Please it.
>
Yes, if sys_pll has a notifier_block instead of cpu_clk, there will be
no problem with the clk_hw API.
I will rework that point.
> >
> >> > +
> >> > +static int meson_a1_cpu_clk_notifier_cb(struct notifier_block *nb,
> >> > + unsigned long event, void *data)
> >> > +{
> >> > + struct a1_cpu_clk_nb_data *nbd;
> >> > + int ret = 0;
> >> > +
> >> > + nbd = container_of(nb, struct a1_cpu_clk_nb_data, nb);
> >> > +
> >> > + switch (event) {
> >> > + case PRE_RATE_CHANGE:
> >> > + nbd->parent = MESON_A1_CPU_CLK_GET_PARENT(nbd);
> >> > + /* Fallback to the CPU fixed clock */
> >> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, 0);
> >> > + /* Wait for clock propagation */
> >> > + udelay(100);
> >> > + break;
> >> > +
> >> > + case POST_RATE_CHANGE:
> >> > + case ABORT_RATE_CHANGE:
> >> > + /* Back to the original parent clock */
> >> > + ret = MESON_A1_CPU_CLK_SET_PARENT(nbd, nbd->parent);
> >> > + /* Wait for clock propagation */
> >> > + udelay(100);
> >> > + break;
> >> > +
> >> > + default:
> >> > + pr_warn("Unknown event %lu for %s notifier block\n",
> >> > + event, clk_hw_get_name(nbd->cpu_clk));
> >> > + break;
> >> > + }
> >> > +
> >> > + return notifier_from_errno(ret);
> >> > +}
> >> > +
> >> > +static struct a1_cpu_clk_nb_data a1_cpu_clk_nb_data = {
> >> > + .mux_ops = &clk_regmap_mux_ops,
> >> > + .cpu_clk = &cpu_clk.hw,
> >> > + .nb.notifier_call = meson_a1_cpu_clk_notifier_cb,
> >> > +};
> >> > +
> >> > +static int meson_a1_dvfs_setup(struct platform_device *pdev)
> >> > +{
> >> > + struct device *dev = &pdev->dev;
> >> > + struct clk *notifier_clk;
> >> > + int ret;
> >> > +
> >> > + /* Setup clock notifier for cpu_clk */
> >> > + notifier_clk = devm_clk_hw_get_clk(dev, &cpu_clk.hw, "dvfs");
> >> > + if (IS_ERR(notifier_clk))
> >> > + return dev_err_probe(dev, PTR_ERR(notifier_clk),
> >> > + "can't get cpu_clk as notifier clock\n");
> >> > +
> >> > + ret = devm_clk_notifier_register(dev, notifier_clk,
> >> > + &a1_cpu_clk_nb_data.nb);
> >> > + if (ret)
> >> > + return dev_err_probe(dev, ret,
> >> > + "can't register cpu_clk notifier\n");
> >> > +
> >> > + return ret;
> >> > +}
> >> > +
> >> > +static int meson_a1_cpu_probe(struct platform_device *pdev)
> >> > +{
> >> > + struct device *dev = &pdev->dev;
> >> > + void __iomem *base;
> >> > + struct regmap *map;
> >> > + int clkid, i, err;
> >> > +
> >> > + base = devm_platform_ioremap_resource(pdev, 0);
> >> > + if (IS_ERR(base))
> >> > + return dev_err_probe(dev, PTR_ERR(base),
> >> > + "can't ioremap resource\n");
> >> > +
> >> > + map = devm_regmap_init_mmio(dev, base, &a1_cpu_regmap_cfg);
> >> > + if (IS_ERR(map))
> >> > + return dev_err_probe(dev, PTR_ERR(map),
> >> > + "can't init regmap mmio region\n");
> >> > +
> >> > + /* Populate regmap for the regmap backed clocks */
> >> > + for (i = 0; i < ARRAY_SIZE(a1_cpu_regmaps); i++)
> >> > + a1_cpu_regmaps[i]->map = map;
> >> > +
> >> > + for (clkid = 0; clkid < a1_cpu_clks.num; clkid++) {
> >> > + err = devm_clk_hw_register(dev, a1_cpu_clks.hws[clkid]);
> >> > + if (err)
> >> > + return dev_err_probe(dev, err,
> >> > + "clock[%d] registration failed\n",
> >> > + clkid);
> >> > + }
> >> > +
> >> > + err = devm_of_clk_add_hw_provider(dev, meson_clk_hw_get, &a1_cpu_clks);
> >> > + if (err)
> >> > + return dev_err_probe(dev, err, "can't add clk hw provider\n");
> >>
> >> I wonder if there is a window of opportunity to poke the syspll without
> >> your notifier here. That being said, the situation would be similar on g12.
> >>
> >
> > Yes, I have taken into account what you did in the G12A CPU clock
> > relations. My thoughts were that it might not be applicable for the A1
> > case. This is because the sys_pll should be located in a different
> > driver from a logical perspective. Consequently, we cannot configure the
> > sys_pll notifier block to manage the cpu_clk from a different driver.
> > However, if I were to move the sys_pll clock object to the A1 CPU clock
> > controller, I believe the g12a sys_pll notifier approach would work.
> >
>
> I fail to see the connection between the number of device and the
> approach you took.
>
>
> >> > +
> >> > + return meson_a1_dvfs_setup(pdev);
> >>
> >>
> >>
> >> > +}
> >> > +
> >> > +static const struct of_device_id a1_cpu_clkc_match_table[] = {
> >> > + { .compatible = "amlogic,a1-cpu-clkc", },
> >> > + {}
> >> > +};
> >> > +MODULE_DEVICE_TABLE(of, a1_cpu_clkc_match_table);
> >> > +
> >> > +static struct platform_driver a1_cpu_clkc_driver = {
> >> > + .probe = meson_a1_cpu_probe,
> >> > + .driver = {
> >> > + .name = "a1-cpu-clkc",
> >> > + .of_match_table = a1_cpu_clkc_match_table,
> >> > + },
> >> > +};
> >> > +
> >> > +module_platform_driver(a1_cpu_clkc_driver);
> >> > +MODULE_AUTHOR("Dmitry Rokosov <ddrokosov@salutedevices.com>");
> >> > +MODULE_LICENSE("GPL");
> >> > diff --git a/drivers/clk/meson/a1-cpu.h b/drivers/clk/meson/a1-cpu.h
> >> > new file mode 100644
> >> > index 000000000000..e9af4117e26f
> >> > --- /dev/null
> >> > +++ b/drivers/clk/meson/a1-cpu.h
> >>
> >> There is not point putting the definition here in a header
> >> These are clearly not going to be shared with another driver.
> >>
> >> Please drop this file
> >>
> >
> > The same approach was applied to the Peripherals and PLL A1 drivers.
> > Honestly, I am not a fan of having different file organization within a
> > single logical code folder.
> >
> > Please refer to:
> >
> > drivers/clk/meson/a1-peripherals.h
> > drivers/clk/meson/a1-pll.h
>
> I understand. There was a time where it made sense, some definition
> could have been shared back then. This is no longer the case and it
> would probably welcome a rework.
>
> ... and again, just pointing to another code does really invalidate my
> point.
>
> >
> >> > @@ -0,0 +1,16 @@
> >> > +/* SPDX-License-Identifier: GPL-2.0+ */
> >> > +/*
> >> > + * Amlogic A1 CPU Clock Controller internals
> >> > + *
> >> > + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> >> > + * Author: Dmitry Rokosov <ddrokosov@salutedevices.com>
> >> > + */
> >> > +
> >> > +#ifndef __A1_CPU_H
> >> > +#define __A1_CPU_H
> >> > +
> >> > +/* cpu clock controller register offset */
> >> > +#define CPUCTRL_CLK_CTRL0 0x80
> >> > +#define CPUCTRL_CLK_CTRL1 0x84
> >>
> >> You are claiming the registers from 0x00 to 0x84 (included), but only
> >> using these 2 registers ? What is the rest ? Are you sure there is only
> >> clocks in there ?
> >>
> >
> > Yes, unfortunately, the register map for this IP is not described in the
> > A1 Datasheet. The only available information about it can be found in
> > the vendor clock driver, which provides details for only two registers
> > used to configure the CPU clock.
> >
> > From vendor kernel dtsi:
> >
> > clkc: clock-controller {
> > compatible = "amlogic,a1-clkc";
> > #clock-cells = <1>;
> > reg = <0x0 0xfe000800 0x0 0x100>,
> > <0x0 0xfe007c00 0x0 0x21c>,
> > <0x0 0xfd000000 0x0 0x88>; <==== CPU clock regmap
> > reg-names = "basic", "pll",
> > "cpu_clk";
> > clocks = <&xtal>;
> > clock-names = "core";
> > status = "okay";
> > };
> >
> > From vendor clkc driver:
> >
> > /*
> > * CPU clok register offset
> > * APB_BASE: APB1_BASE_ADDR = 0xfd000000
> > */
> >
> > #define CPUCTRL_CLK_CTRL0 0x80
> > #define CPUCTRL_CLK_CTRL1 0x84
>
> If you had clock in 0x0 and 0x80, then I would agree claiming 0x0-0x88
> is reasonable, even if you don't really know what is in between. It
> would be a fair assumption.
>
> It is not the case here.
> For all we know it could resets, power domains, etc ...
>
However, we do not have any information indicating that the gap from
0x00-0x80 contains additional registers. It is possible that there are
internal registers, but they are not mentioned in the vendor datasheet
or driver. Therefore, in this case, I personally prefer to rely on the
vendor mapping.
--
Thank you,
Dmitry
^ permalink raw reply
* [PATCH 2/2] arm64: dts: ti: k3-am642-phyboard-electra-rdk: Increase CAN max bitrate
From: Nathan Morrisson @ 2024-04-02 16:08 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
In-Reply-To: <20240402160825.1516036-1-nmorrisson@phytec.com>
The phyBOARD-Electra has two TCAN1044VDD CAN transceivers which
support CAN FD at 8 Mbps.
Increase the maximum bitrate to 8 Mbps.
Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
---
arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
index 8237b8c815b8..522699ec65e8 100644
--- a/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts
@@ -42,7 +42,7 @@ can_tc1: can-phy0 {
pinctrl-names = "default";
pinctrl-0 = <&can_tc1_pins_default>;
#phy-cells = <0>;
- max-bitrate = <5000000>;
+ max-bitrate = <8000000>;
standby-gpios = <&main_gpio0 32 GPIO_ACTIVE_HIGH>;
};
@@ -51,7 +51,7 @@ can_tc2: can-phy1 {
pinctrl-names = "default";
pinctrl-0 = <&can_tc2_pins_default>;
#phy-cells = <0>;
- max-bitrate = <5000000>;
+ max-bitrate = <8000000>;
standby-gpios = <&main_gpio0 35 GPIO_ACTIVE_HIGH>;
};
--
2.25.1
^ permalink raw reply related
* [PATCH 0/2] Increase CAN max bitrate for phyCORE-AM62x and phyCORE-am64x
From: Nathan Morrisson @ 2024-04-02 16:08 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
Nathan Morrisson (2):
arm64: dts: ti: k3-am625-phyboard-lyra-rdk: Increase CAN max bitrate
arm64: dts: ti: k3-am642-phyboard-electra-rdk: Increase CAN max
bitrate
arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts | 2 +-
arch/arm64/boot/dts/ti/k3-am642-phyboard-electra-rdk.dts | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH 1/2] arm64: dts: ti: k3-am625-phyboard-lyra-rdk: Increase CAN max bitrate
From: Nathan Morrisson @ 2024-04-02 16:08 UTC (permalink / raw)
To: nm, vigneshr, kristo, robh, krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-kernel, devicetree, linux-kernel, upstream, w.egorov
In-Reply-To: <20240402160825.1516036-1-nmorrisson@phytec.com>
The phyBOARD-Lyra has one TCAN1044VDD CAN transceiver which supports
CAN FD at 8 Mbps.
Increase the maximum bitrate to 8 Mbps.
Signed-off-by: Nathan Morrisson <nmorrisson@phytec.com>
---
arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts b/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
index a83a90497857..e225d76d02c8 100644
--- a/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am625-phyboard-lyra-rdk.dts
@@ -31,7 +31,7 @@ aliases {
can_tc1: can-phy0 {
compatible = "ti,tcan1042";
#phy-cells = <0>;
- max-bitrate = <5000000>;
+ max-bitrate = <8000000>;
standby-gpios = <&gpio_exp 1 GPIO_ACTIVE_HIGH>;
};
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: clock: qcom: Update SM8150 videocc bindings
From: Rob Herring @ 2024-04-02 16:05 UTC (permalink / raw)
To: Satya Priya Kakitapalli
Cc: Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley,
Michael Turquette, Stephen Boyd, Dmitry Baryshkov, Ajit Pandey,
Imran Shaik, Taniya Das, Jagadeesh Kona, linux-arm-msm,
devicetree, linux-kernel, linux-clk
In-Reply-To: <20240401-videocc-sm8150-dt-node-v2-1-3b87cd2add96@quicinc.com>
On Mon, Apr 01, 2024 at 04:44:23PM +0530, Satya Priya Kakitapalli wrote:
> Update the clocks list for SM8150 to add both AHB and XO clocks,
> as it needs both of them.
I read this as you are adding 2 clocks, but it is really just 1 you are
adding (iface).
This should have more detail on why breaking the ABI is okay here.
>
> Fixes: 35d26e9292e2 ("dt-bindings: clock: Add YAML schemas for the QCOM VIDEOCC clock bindings")
> Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@quicinc.com>
> ---
> .../devicetree/bindings/clock/qcom,videocc.yaml | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> index 6999e36ace1b..68bac801adb0 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,videocc.yaml
> @@ -75,7 +75,6 @@ allOf:
> enum:
> - qcom,sc7180-videocc
> - qcom,sdm845-videocc
> - - qcom,sm8150-videocc
> then:
> properties:
> clocks:
> @@ -101,6 +100,22 @@ allOf:
> - const: bi_tcxo
> - const: bi_tcxo_ao
>
> + - if:
> + properties:
> + compatible:
> + enum:
> + - qcom,sm8150-videocc
> + then:
> + properties:
> + clocks:
> + items:
> + - description: AHB
> + - description: Board XO source
> + clock-names:
> + items:
> + - const: iface
> + - const: bi_tcxo
> +
> - if:
> properties:
> compatible:
>
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: stmmac: add support for RZ/N1 GMAC
From: Romain Gantois @ 2024-04-02 16:04 UTC (permalink / raw)
To: Russell King (Oracle)
Cc: Romain Gantois, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Clément Léger, Thomas Petazzoni,
netdev, devicetree, linux-kernel, linux-renesas-soc, linux-stm32,
linux-arm-kernel
In-Reply-To: <ZgwoygldsA1V8fs9@shell.armlinux.org.uk>
Hello Russell,
On Tue, 2 Apr 2024, Russell King (Oracle) wrote:
> > I'm afraid that this fails at one of the most basic principles of kernel
> > multi-threaded programming. stmmac_dvr_probe() as part of its work calls
> > register_netdev() which publishes to userspace the network device.
> >
> > Everything that is required must be setup _prior_ to publication to
> > userspace to avoid races, because as soon as the network device is
> > published, userspace can decide to bring that interface up. If one
> > hasn't finished the initialisation, the interface can be brought up
> > before that initialisation is complete.
...
>
> I'm not going to say that the two patches threaded to this email are
> "sane" but at least it avoids the problem. socfpga still has issues
> with initialisation done after register_netdev() though.
Thanks a lot for providing a fix to this issue, introducing new pcs_init/exit()
hooks seems like the best solution at this time, I'll make sure to integrate
those patches in the v2 for this series.
Thanks,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH V5] PCI: Add support for preserving boot configuration
From: Rob Herring @ 2024-04-02 16:01 UTC (permalink / raw)
To: Vidya Sagar
Cc: lpieralisi, mmaddireddy, linux-kernel, will, jonathanh, kthota,
frowand.list, kw, linux-arm-kernel, lenb, devicetree, sagar.tv,
rafael, bhelgaas, linux-pci, treding, linux-acpi
In-Reply-To: <20240401075031.3337211-1-vidyas@nvidia.com>
On Mon, 01 Apr 2024 13:20:31 +0530, Vidya Sagar wrote:
> Add support for preserving the boot configuration done by the
> platform firmware per host bridge basis, based on the presence of
> 'linux,pci-probe-only' property in the respective PCI host bridge
> device-tree node. It also unifies the ACPI and DT based boot flows
> in this regard.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V5:
> * Addressed Rob's review comments
>
> V4:
> * Addressed Bjorn's review comments
>
> V3:
> * Unified ACPI and DT flows as part of addressing Bjorn's review comments
>
> V2:
> * Addressed issues reported by kernel test robot <lkp@intel.com>
>
> drivers/acpi/pci_root.c | 12 -----
> drivers/pci/controller/pci-host-common.c | 4 --
> drivers/pci/of.c | 57 +++++++++++++++++++-----
> drivers/pci/probe.c | 46 ++++++++++++++-----
> include/linux/of_pci.h | 6 +++
> 5 files changed, 88 insertions(+), 37 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: msm8996: add fastrpc nodes
From: Dmitry Baryshkov @ 2024-04-02 15:58 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Mathieu Poirier, Sibi Sankar, linux-arm-msm, devicetree,
linux-remoteproc, linux-kernel, Srinivas Kandagatla
In-Reply-To: <d9ba1e11-44ea-4c1f-ab33-56a8bf57ab63@linaro.org>
On Tue, 2 Apr 2024 at 17:47, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 31.03.2024 11:10 PM, Dmitry Baryshkov wrote:
> > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> >
> > The ADSP provides fastrpc/compute capabilities. Enable support for the
> > fastrpc on this DSP.
> >
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > arch/arm64/boot/dts/qcom/msm8996.dtsi | 57 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > index 7ae499fa7d91..cf7ab01f3af6 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > @@ -3545,6 +3545,63 @@ q6routing: routing {
> > };
> > };
> > };
> > +
> > + fastrpc {
> > + compatible = "qcom,fastrpc";
> > + qcom,smd-channels = "fastrpcsmd-apps-dsp";
> > + label = "adsp";
> > + qcom,non-secure-domain;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cb@8 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <8>;
> > + iommus = <&lpass_q6_smmu 8>;
> > + };
> > +
> > + cb@9 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <9>;
> > + iommus = <&lpass_q6_smmu 9>;
> > + };
> > +
> > + cb@10 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <10>;
> > + iommus = <&lpass_q6_smmu 10>;
> > + };
> > +
> > + cb@11 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <11>;
> > + iommus = <&lpass_q6_smmu 11>;
> > + };
> > +
> > + cb@12 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <12>;
> > + iommus = <&lpass_q6_smmu 12>;
> > + };
> > +
> > + cb@5 {
> > + compatible = "qcom,fastrpc-compute-cb";
> > + reg = <5>;
>
> No need to copy downstream's creative alphabetical-but-not-numerical
> sorting..
Ack, I'll fix the order.
> The entries look OK though.. although, any reason we have
> such a weird binding including faux child nodes and not just an array
> of iommus? Is the only way to discover the fastrpc nodes' properties
> such as qcom,non-secure-domain or vmid belonging through hardcoding?
No idea here. This is how fastrpc nodes are defined on all existing
platforms. Maybe Srini knows the story and the reason behind the
bindings??
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v6 3/4] firmware: arm_scmi: Add SCMI v3.2 pincontrol protocol basic support
From: Cristian Marussi @ 2024-04-02 15:58 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Peng Fan, Peng Fan (OSS), Sudeep Holla, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij, Dan Carpenter,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-gpio@vger.kernel.org, Oleksii Moisieiev
In-Reply-To: <CAHp75VdAaTeQ_Ag3gd0s9UfT=kAT2hwibeJ9-YFXJx4z=R3e+g@mail.gmail.com>
On Tue, Apr 02, 2024 at 04:06:06PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 2, 2024 at 10:48 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
> > On Sun, Mar 31, 2024 at 01:44:28PM +0000, Peng Fan wrote:
> > > > Sat, Mar 23, 2024 at 08:15:16PM +0800, Peng Fan (OSS) kirjoitti:
>
> ...
>
> > > > > +#include <linux/module.h>
> > > > > +#include <linux/scmi_protocol.h>
> > > > > +#include <linux/slab.h>
> > > >
> > > > This is semi-random list of headers. Please, follow IWYU principle (include
> > > > what you use). There are a lot of inclusions I see missing (just in the context of
> > > > this page I see bits.h, types.h, and asm/byteorder.h).
> > >
> > > Is there any documentation about this requirement?
> > > Some headers are already included by others.
>
> The documentation here is called "a common sense".
> The C language is built like this and we expect that nobody will
> invest into the dependency hell that we have already, that's why IWYU
> principle, please follow it.
>
Yes, but given that we have a growing number of SCMI protocols there is a
common local protocols.h header to group all includes needed by any
protocols: the idea behind this (and the devm_ saga down below) was to ease
development of protocols, since there are lots of them and growing, given
the SCMI spec is extensible.
> > Andy made (mostly) the same remarks on this same patch ~1-year ago on
> > this same patch while it was posted by Oleksii.
> >
> > And I told that time that most of the remarks around devm_ usage were
> > wrong due to how the SCMI core handles protocol initialization (using a
> > devres group transparently).
> >
> > This is what I answered that time.
> >
> > https://lore.kernel.org/linux-arm-kernel/ZJ78hBcjAhiU+ZBO@e120937-lin/#t
> >
> > I wont repeat myself, but, in a nutshell the memory allocation like it
> > is now is fine: a bit happens via devm_ at protocol initialization, the
> > other is doe via explicit kmalloc at runtime and freed via kfree at
> > remove time (if needed...i.e. checking the present flag of some structs)
>
> This sounds like a mess. devm_ is expected to be used only for the
> ->probe() stage, otherwise you may consider cleanup.h (__free() macro)
> to have automatic free at the paths where memory is not needed.
>
Indeed, this protocol_init code is called by the SCMI core once for all when
an SCMI driver tries at first to use this specific protocol by 'getting' its
protocol_ops, so it is indeed called inside the probe chain of the driver:
at this point you *can* decide to use devres to allocate memory and be assured
that if the init fails, or when the driver cease to use this protocol (calling
its remove()) and no other driver is using it, all the stuff that have been
allocated related to this protocol will be released by the core for you.
(using an internal devres group)
Without this you should handle manually all the deallocation manually on
the init error-paths AND also provide all the cleanup explicitly when
the protocol is no more used by any driver (multiple users of the same
protocol instance are possible)...for all protocols.
This is/was handy since, till now, all the SCMI querying and resources
allocation happened anyway all at once at init time...
...the mess, as you kindly called it, derives from the fact that this specific
protocol is the first and only one that does NOT allocate all that it needs
during the initialization (to minimize needless allocs for a lot of possibly
unused resources) and this lazy-initialization phase, done after init at runtime,
must be handled manually since it cannot be managed by the devres group that is
open/clsoed around init by the SCMI core.
I dont like particularly this split allocation but it has a reason and any
other solution seems more messy to me at the moment.
And I dont feel like changing all the SCMI protocol initialziation core code
(that address a lot more under the hood) is a desirable solution to address a
non-existent problem really.
> And the function naming doesn't suggest that you have a probe-remove
> pair. Moreover, if the init-deinit part is called in the probe-remove,
> the devm_ must not be mixed with non-devm ones, as it breaks the order
> and leads to subtle mistakes.
>
Initialization order is enforced by SCMI core like this:
@driver_probe->get_protocol_ops()
@core/get_protocol_ops
-> devres_group_open()
-> protocol_init->devm_*()
-> devres_group_close()
-> driver_probing
@runtime optional explicit_lazy_kmallocs inside the protocol
@driver_remove->put_protocol_ops()
@core/put_protocol_ops()
-> protocol_denit->optional_explicit_kfree_of_the_above
-> devres_group_release()
-> driver_removing
... dont think there's an ordering problem.
...note that the ph->dev provided in the protocol_init and used by devm_
is NOT the dev of the SCMI driver probe/remove that uses the get_protocol_ops,
it is an internal SCMI device associated with the core SCMI stack probing and
allocations, within which a devres group for the specific protocol is created
when that specific protocol is initialized...protocols are not fully
fledged drivers are just bits of the SCMI stack that are initialized when needed
(and possibly also loaded when needed for vendor protocols) and
de-initialzed when no more SCMI driver users exist for that protocol.
Thanks,
Cristian
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Dmitry Baryshkov @ 2024-04-02 15:55 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
ath10k, wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson,
Jami Kettunen, Jeffrey Hugo
In-Reply-To: <0ca1221b-b707-450f-877d-ca07a601624d@freebox.fr>
On Tue, 2 Apr 2024 at 18:31, Marc Gonzalez <mgonzalez@freebox.fr> wrote:
>
> On 02/04/2024 16:34, Konrad Dybcio wrote:
>
> > On 30.03.2024 7:25 PM, Krzysztof Kozlowski wrote:
> >
> >> On 28/03/2024 18:39, Marc Gonzalez wrote:
> >>
> >>> The ath10k driver waits for an "MSA_READY" indicator
> >>> to complete initialization. If the indicator is not
> >>> received, then the device remains unusable.
> >>>
> >>> cf. ath10k_qmi_driver_event_work()
> >>>
> >>> Several msm8998-based devices are affected by this issue.
> >>> Oddly, it seems safe to NOT wait for the indicator, and
> >>> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
> >>>
> >>> Jeff Johnson wrote:
> >>>
> >>> The feedback I received was "it might be ok to change all ath10k qmi
> >>> to skip waiting for msa_ready", and it was pointed out that ath11k
> >>> (and ath12k) do not wait for it.
> >>>
> >>> However with so many deployed devices, "might be ok" isn't a strong
> >>> argument for changing the default behavior.
> >>
> >> I think you got pretty clear comments:
> >>
> >> "This sounds more like a firmware feature, not a hardware feature."
> >>
> >> "This is why having this property in DT does not look right
> >> place for this."
> >
> > Translating from dt maintainer speak to English, a functionally-equivalent
> > resolution of adding an of_machine_is_compatible("qcom,msm8998") is more
> > in line with the guidelines of not sprinkling firmware specifics in DTs
>
> I'm not so sure about that, as I had proposed
>
> + if (of_device_is_compatible(of_root, "qcom,msm8998")
> + qmi->no_point_in_waiting_for_msa_ready_indicator = true;
> +
>
> To which Conor replied:
>
> > How come the root node comes into this, don't you have a soc-specific
> > compatible for the integration on this SoC?
> > (I am assuming that this is not the SDIO variant, given then it'd not be
> > fixed to this particular implementation)
>
>
> Then added:
>
> > A SoC-specific compatible sounds like it would be suitable in that case
> > then, to deal with integration quirks for that specific SoC? I usually
> > leave the ins and outs of these qcom SoCs to Krzysztof, but I can't help
> > but wanna know what the justification is here for not using one.
>
>
> Then Krzysztof added:
>
> > The WiFi+BT chips are separate products, so they are not usually
> > considered part of the SoC, even though they can be integrated into the
> > SoC like here. I guess correct approach would be to add SoC-specific
> > compatible for them.
>
>
> So, if I understand correctly, I take this to mean that I should:
>
> 1) DELETE the qcom,no-msa-ready-indicator boolean property,
> 2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
I'd say, this is not correct. There is no "msm8998-wifi".
> 3) ADD that compatible to the wifi node in msm8998.dtsi
> compatible = "qcom,wcn3990-wifi", "qcom,msm8998-wifi";
> 4) In the driver, set qmi->fake_msa_ready_indicator to true if we detect "qcom,msm8998-wifi"
>
> And this approach would be acceptable to both ath10k & DT maintainers?
I'd say, we should take a step back and actually verify how this was
handled in the vendor kernel.
>
> Bjarne, Konrad: is it OK to apply the work-around for all msm8998 boards?
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH net-next 2/2] net: stmmac: dwmac-socfpga: use pcs_init/pcs_exit
From: Russell King (Oracle) @ 2024-04-02 15:51 UTC (permalink / raw)
To: Romain Gantois
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Clément Léger, Thomas Petazzoni,
netdev, devicetree, linux-kernel, linux-renesas-soc, linux-stm32,
linux-arm-kernel
In-Reply-To: <ZgwoygldsA1V8fs9@shell.armlinux.org.uk>
Use the newly introduced pcs_init() and pcs_exit() operations to
create and destroy the PCS instance at a more appropriate moment during
the driver lifecycle, thereby avoiding publishing a network device to
userspace that has not yet finished its PCS initialisation.
There are other similar issues with this driver which remain
unaddressed, but these are out of scope for this patch.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 109 +++++++++---------
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 12b4a80ea3aa..67ca163936c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -379,6 +379,58 @@ static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac)
return 0;
}
+static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv,
+ struct mac_device_info *hw)
+{
+ struct socfpga_dwmac *dwmac = priv->plat->bsp_priv;
+ struct regmap_config pcs_regmap_cfg = {
+ .reg_bits = 16,
+ .val_bits = 16,
+ .reg_shift = regmap_upshift(1),
+ };
+ struct mdio_regmap_config mrc;
+ struct regmap *pcs_regmap;
+ struct phylink_pcs *pcs;
+ struct mii_bus *pcs_bus;
+
+ if (!dwmac->tse_pcs_base)
+ return 0;
+
+ pcs_regmap = devm_regmap_init_mmio(priv->device, dwmac->tse_pcs_base,
+ &pcs_regmap_cfg);
+ if (IS_ERR(pcs_regmap))
+ return PTR_ERR(pcs_regmap);
+
+ memset(&mrc, 0, sizeof(mrc));
+ mrc.regmap = pcs_regmap;
+ mrc.parent = priv->device;
+ mrc.valid_addr = 0x0;
+ mrc.autoscan = false;
+
+ /* Can't use ndev->name here because it will not have been initialised,
+ * and in any case, the user can rename network interfaces at runtime.
+ */
+ snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii",
+ dev_name(priv->device));
+ pcs_bus = devm_mdio_regmap_register(priv->device, &mrc);
+ if (IS_ERR(pcs_bus))
+ return PTR_ERR(pcs_bus);
+
+ pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
+ if (IS_ERR(pcs))
+ return PTR_ERR(pcs);
+
+ hw->phylink_pcs = pcs;
+ return 0;
+}
+
+static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv,
+ struct mac_device_info *hw)
+{
+ if (hw->phylink_pcs)
+ lynx_pcs_destroy(hw->phylink_pcs);
+}
+
static int socfpga_dwmac_probe(struct platform_device *pdev)
{
struct plat_stmmacenet_data *plat_dat;
@@ -426,6 +478,8 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
dwmac->ops = ops;
plat_dat->bsp_priv = dwmac;
plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
+ plat_dat->pcs_init = socfpga_dwmac_pcs_init;
+ plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (ret)
@@ -444,48 +498,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
if (ret)
goto err_dvr_remove;
- /* Create a regmap for the PCS so that it can be used by the PCS driver,
- * if we have such a PCS
- */
- if (dwmac->tse_pcs_base) {
- struct regmap_config pcs_regmap_cfg;
- struct mdio_regmap_config mrc;
- struct regmap *pcs_regmap;
- struct mii_bus *pcs_bus;
-
- memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
- memset(&mrc, 0, sizeof(mrc));
-
- pcs_regmap_cfg.reg_bits = 16;
- pcs_regmap_cfg.val_bits = 16;
- pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
-
- pcs_regmap = devm_regmap_init_mmio(&pdev->dev, dwmac->tse_pcs_base,
- &pcs_regmap_cfg);
- if (IS_ERR(pcs_regmap)) {
- ret = PTR_ERR(pcs_regmap);
- goto err_dvr_remove;
- }
-
- mrc.regmap = pcs_regmap;
- mrc.parent = &pdev->dev;
- mrc.valid_addr = 0x0;
- mrc.autoscan = false;
-
- snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
- pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
- if (IS_ERR(pcs_bus)) {
- ret = PTR_ERR(pcs_bus);
- goto err_dvr_remove;
- }
-
- stpriv->hw->phylink_pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
- if (IS_ERR(stpriv->hw->phylink_pcs)) {
- ret = PTR_ERR(stpriv->hw->phylink_pcs);
- goto err_dvr_remove;
- }
- }
-
return 0;
err_dvr_remove:
@@ -494,17 +506,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
return ret;
}
-static void socfpga_dwmac_remove(struct platform_device *pdev)
-{
- struct net_device *ndev = platform_get_drvdata(pdev);
- struct stmmac_priv *priv = netdev_priv(ndev);
- struct phylink_pcs *pcs = priv->hw->phylink_pcs;
-
- stmmac_pltfr_remove(pdev);
-
- lynx_pcs_destroy(pcs);
-}
-
#ifdef CONFIG_PM_SLEEP
static int socfpga_dwmac_resume(struct device *dev)
{
@@ -576,7 +577,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
static struct platform_driver socfpga_dwmac_driver = {
.probe = socfpga_dwmac_probe,
- .remove_new = socfpga_dwmac_remove,
+ .remove_new = stmmac_pltfr_remove,
.driver = {
.name = "socfpga-dwmac",
.pm = &socfpga_dwmac_pm_ops,
--
2.30.2
^ permalink raw reply related
* [PATCH net-next 1/2] net: stmmac: introduce pcs_init/pcs_exit stmmac operations
From: Russell King (Oracle) @ 2024-04-02 15:51 UTC (permalink / raw)
To: Romain Gantois
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Clément Léger, Thomas Petazzoni,
netdev, devicetree, linux-kernel, linux-renesas-soc, linux-stm32,
linux-arm-kernel
In-Reply-To: <ZgwoygldsA1V8fs9@shell.armlinux.org.uk>
Introduce a mechanism whereby platforms can create their PCS instances
prior to the network device being published to userspace, but after
some of the core stmmac initialisation has been completed. This means
that the data structures that platforms need will be available.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
include/linux/stmmac.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fe3498e86de9..25fa33ae7017 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7208,6 +7208,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
if (ret)
return ret;
+ if (priv->plat->pcs_init) {
+ ret = priv->plat->pcs_init(priv, priv->hw);
+ if (ret)
+ return ret;
+ }
+
/* Get the HW capability (new GMAC newer than 3.50a) */
priv->hw_cap_support = stmmac_get_hw_features(priv);
if (priv->hw_cap_support) {
@@ -7290,6 +7296,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
return 0;
}
+static void stmmac_hw_exit(struct stmmac_priv *priv)
+{
+ if (priv->plat->pcs_exit)
+ priv->plat->pcs_exit(priv, priv->hw);
+}
+
static void stmmac_napi_add(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
@@ -7804,6 +7816,7 @@ int stmmac_dvr_probe(struct device *device,
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
error_mdio_register:
+ stmmac_hw_exit(priv);
stmmac_napi_del(ndev);
error_hw_init:
destroy_workqueue(priv->wq);
@@ -7844,6 +7857,7 @@ void stmmac_dvr_remove(struct device *dev)
if (priv->hw->pcs != STMMAC_PCS_TBI &&
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
+ stmmac_hw_exit(priv);
destroy_workqueue(priv->wq);
mutex_destroy(&priv->lock);
bitmap_free(priv->af_xdp_zc_qps);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756..941fde506e51 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -285,6 +285,8 @@ struct plat_stmmacenet_data {
int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
void *ctx);
void (*dump_debug_regs)(void *priv);
+ int (*pcs_init)(struct stmmac_priv *priv, struct mac_device_info *hw);
+ void (*pcs_exit)(struct stmmac_priv *priv, struct mac_device_info *hw);
void *bsp_priv;
struct clk *stmmac_clk;
struct clk *pclk;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH net-next 2/3] net: stmmac: add support for RZ/N1 GMAC
From: Russell King (Oracle) @ 2024-04-02 15:48 UTC (permalink / raw)
To: Romain Gantois
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm, Alexandre Torgue, Jose Abreu,
Maxime Coquelin, Clément Léger, Thomas Petazzoni,
netdev, devicetree, linux-kernel, linux-renesas-soc, linux-stm32,
linux-arm-kernel
In-Reply-To: <ZgwM/FIKTuN4vkQA@shell.armlinux.org.uk>
On Tue, Apr 02, 2024 at 02:49:48PM +0100, Russell King (Oracle) wrote:
> On Tue, Apr 02, 2024 at 02:37:01PM +0200, Romain Gantois wrote:
> > + ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> > + if (ret)
> > + return ret;
> > +
> > + ndev = platform_get_drvdata(pdev);
> > + priv = netdev_priv(ndev);
> > +
> > + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> > + if (pcs_node) {
> > + pcs = miic_create(dev, pcs_node);
> > + of_node_put(pcs_node);
> > + if (IS_ERR(pcs))
> > + return PTR_ERR(pcs);
> > +
> > + priv->hw->phylink_pcs = pcs;
> > + }
>
> I'm afraid that this fails at one of the most basic principles of kernel
> multi-threaded programming. stmmac_dvr_probe() as part of its work calls
> register_netdev() which publishes to userspace the network device.
>
> Everything that is required must be setup _prior_ to publication to
> userspace to avoid races, because as soon as the network device is
> published, userspace can decide to bring that interface up. If one
> hasn't finished the initialisation, the interface can be brought up
> before that initialisation is complete.
>
> I don't see anything obvious in the stmmac data structures that would
> allow you to hook in at an appropriate point before the
> register_netdev() but after the netdev has been created. The
> priv->hw data structure is created by stmmac_hwif_init()
>
> I see that drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c is also
> guilty of this as well, and should be fixed. It's even worse because it
> does a truck load of stuff after stmmac_dvr_probe() which it most
> definitely should not be doing.
>
> I definitely get the feeling that the structure of the stmmac driver
> is really getting out of hand, and is making stuff harder for people,
> and it's not improving over time - in fact, it's getting worse. It
> needs a *lot* of work to bring it back to a sane model.
I'm not going to say that the two patches threaded to this email are
"sane" but at least it avoids the problem. socfpga still has issues
with initialisation done after register_netdev() though.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs
From: Oleksij Rempel @ 2024-04-02 15:47 UTC (permalink / raw)
To: Rob Herring
Cc: Kory Maincent, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Luis Chamberlain, Russ Weight,
Greg Kroah-Hartman, Rafael J. Wysocki, Krzysztof Kozlowski,
Conor Dooley, Mark Brown, Frank Rowand, Andrew Lunn,
Heiner Kallweit, Russell King, Thomas Petazzoni, netdev,
linux-kernel, linux-doc, devicetree, Dent Project
In-Reply-To: <20240402132637.GA3744978-robh@kernel.org>
On Tue, Apr 02, 2024 at 08:26:37AM -0500, Rob Herring wrote:
> > + pairsets:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + description:
> > + List of phandles, each pointing to the power supply for the
> > + corresponding pairset named in 'pairset-names'. This property
> > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4.
> > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133)
> > + |-----------|---------------|---------------|---------------|---------------|
> > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B |
> > + | | (MDI-X) | (MDI) | (X) | (S) |
> > + |-----------|---------------|---------------|---------------|---------------|
> > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 |
> > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 |
> > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 |
> > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE |
> > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE |
> > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 |
> > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE |
> > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE |
> > + minItems: 1
> > + maxItems: 2
>
> "pairsets" does not follow the normal design pattern of foos, foo-names,
> and #foo-cells. You could add #foo-cells I suppose, but what would cells
> convey? I don't think it's a good fit for what you need.
>
> The other oddity is the number of entries and the names are fixed. That
> is usually defined per consumer.
>
> As each entry is just a power rail, why can't the regulator binding be
> used here?
I'm not against describing it consequent with regulator till the wire
end, but right now I have no idea how it should be described by using
regulator bindings. There are maximum 2 rails going in to PSE PI on one
side and 4 rails with at least 5 combinations supported by standard on
other side. Instead of inventing anything new, I suggested to describe
supported output combinations by using IEEE 802.3 standard.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Marc Gonzalez @ 2024-04-02 15:31 UTC (permalink / raw)
To: Konrad Dybcio, Krzysztof Kozlowski, Kalle Valo, Jeff Johnson,
ath10k
Cc: wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Jami Kettunen,
Jeffrey Hugo, Dmitry Baryshkov
In-Reply-To: <502322f1-4f66-4922-bc4e-46bacac23410@linaro.org>
On 02/04/2024 16:34, Konrad Dybcio wrote:
> On 30.03.2024 7:25 PM, Krzysztof Kozlowski wrote:
>
>> On 28/03/2024 18:39, Marc Gonzalez wrote:
>>
>>> The ath10k driver waits for an "MSA_READY" indicator
>>> to complete initialization. If the indicator is not
>>> received, then the device remains unusable.
>>>
>>> cf. ath10k_qmi_driver_event_work()
>>>
>>> Several msm8998-based devices are affected by this issue.
>>> Oddly, it seems safe to NOT wait for the indicator, and
>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
>>>
>>> Jeff Johnson wrote:
>>>
>>> The feedback I received was "it might be ok to change all ath10k qmi
>>> to skip waiting for msa_ready", and it was pointed out that ath11k
>>> (and ath12k) do not wait for it.
>>>
>>> However with so many deployed devices, "might be ok" isn't a strong
>>> argument for changing the default behavior.
>>
>> I think you got pretty clear comments:
>>
>> "This sounds more like a firmware feature, not a hardware feature."
>>
>> "This is why having this property in DT does not look right
>> place for this."
>
> Translating from dt maintainer speak to English, a functionally-equivalent
> resolution of adding an of_machine_is_compatible("qcom,msm8998") is more
> in line with the guidelines of not sprinkling firmware specifics in DTs
I'm not so sure about that, as I had proposed
+ if (of_device_is_compatible(of_root, "qcom,msm8998")
+ qmi->no_point_in_waiting_for_msa_ready_indicator = true;
+
To which Conor replied:
> How come the root node comes into this, don't you have a soc-specific
> compatible for the integration on this SoC?
> (I am assuming that this is not the SDIO variant, given then it'd not be
> fixed to this particular implementation)
Then added:
> A SoC-specific compatible sounds like it would be suitable in that case
> then, to deal with integration quirks for that specific SoC? I usually
> leave the ins and outs of these qcom SoCs to Krzysztof, but I can't help
> but wanna know what the justification is here for not using one.
Then Krzysztof added:
> The WiFi+BT chips are separate products, so they are not usually
> considered part of the SoC, even though they can be integrated into the
> SoC like here. I guess correct approach would be to add SoC-specific
> compatible for them.
So, if I understand correctly, I take this to mean that I should:
1) DELETE the qcom,no-msa-ready-indicator boolean property,
2) ADD a "qcom,msm8998-wifi" (name OK?) compatible,
3) ADD that compatible to the wifi node in msm8998.dtsi
compatible = "qcom,wcn3990-wifi", "qcom,msm8998-wifi";
4) In the driver, set qmi->fake_msa_ready_indicator to true if we detect "qcom,msm8998-wifi"
And this approach would be acceptable to both ath10k & DT maintainers?
Bjarne, Konrad: is it OK to apply the work-around for all msm8998 boards?
Regards
^ permalink raw reply
* Re: [RFC PATCH v2 4/5] clk: meson: a1: add the audio clock controller driver
From: Jerome Brunet @ 2024-04-02 15:11 UTC (permalink / raw)
To: Jan Dakinevich
Cc: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
Martin Blumenstingl, Philipp Zabel, linux-amlogic, linux-clk,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240328010831.884487-5-jan.dakinevich@salutedevices.com>
On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> This controller provides clocks and reset functionality for audio
> peripherals on Amlogic A1 SoC family.
>
> The driver is almost identical to 'axg-audio', however it would be better
> to keep it separate due to following reasons:
>
> - significant amount of bits has another definition. I will bring there
> a mess of new defines with A1_ suffixes.
>
> - registers of this controller are located in two separate regions. It
> will give a lot of complications for 'axg-audio' to support this.
>
> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> ---
> drivers/clk/meson/Kconfig | 13 +
> drivers/clk/meson/Makefile | 1 +
> drivers/clk/meson/a1-audio.c | 624 +++++++++++++++++++++++++++++++++++
> drivers/clk/meson/a1-audio.h | 45 +++
> 4 files changed, 683 insertions(+)
> create mode 100644 drivers/clk/meson/a1-audio.c
> create mode 100644 drivers/clk/meson/a1-audio.h
>
> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> index d6a2fa5f7e88..80c4a18c83d2 100644
> --- a/drivers/clk/meson/Kconfig
> +++ b/drivers/clk/meson/Kconfig
> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
> controller to work.
>
> +config COMMON_CLK_A1_AUDIO
> + tristate "Amlogic A1 SoC Audio clock controller support"
> + depends on ARM64
> + select COMMON_CLK_MESON_REGMAP
> + select COMMON_CLK_MESON_CLKC_UTILS
> + select COMMON_CLK_MESON_PHASE
> + select COMMON_CLK_MESON_SCLK_DIV
> + select COMMON_CLK_MESON_AUDIO_RSTC
> + help
> + Support for the Audio clock controller on Amlogic A113L based
> + device, A1 SoC Family. Say Y if you want A1 Audio clock controller
> + to work.
> +
> config COMMON_CLK_G12A
> tristate "G12 and SM1 SoC clock controllers support"
> depends on ARM64
> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
> index 88d94921a4dc..4968fc7ad555 100644
> --- a/drivers/clk/meson/Makefile
> +++ b/drivers/clk/meson/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
> new file mode 100644
> index 000000000000..bd2b6dde75d4
> --- /dev/null
> +++ b/drivers/clk/meson/a1-audio.c
> @@ -0,0 +1,624 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> + *
> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/init.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +
> +#include "meson-clkc-utils.h"
> +#include "meson-audio-rstc.h"
> +#include "meson-audio.h"
> +#include "clk-regmap.h"
> +#include "clk-phase.h"
> +#include "sclk-div.h"
> +#include "a1-audio.h"
> +
> +static const struct clk_parent_data a1_pclk_pdata[] = {
> + { .fw_name = "pclk" },
> +};
Shouldn't you go through AUD2_CLKID_AUDIOTOP instead ?
> +
> +#define AUD_PCLK_GATE(_name, _reg, _bit) { \
> + .data = &(struct clk_regmap_gate_data){ \
> + .offset = (_reg), \
> + .bit_idx = (_bit), \
> + }, \
> + .hw.init = &(struct clk_init_data) { \
> + .name = "aud_"#_name, \
> + .ops = &clk_regmap_gate_ops, \
> + .parent_data = a1_pclk_pdata, \
> + .num_parents = 1, \
> + }, \
> +}
> +
> +struct clk_regmap aud_ddr_arb =
> + AUD_PCLK_GATE(ddr_arb, AUDIO_CLK_GATE_EN0, 0);
> +struct clk_regmap aud_tdmin_a =
> + AUD_PCLK_GATE(tdmin_a, AUDIO_CLK_GATE_EN0, 1);
> +struct clk_regmap aud_tdmin_b =
> + AUD_PCLK_GATE(tdmin_b, AUDIO_CLK_GATE_EN0, 2);
> +struct clk_regmap aud_tdmin_lb =
> + AUD_PCLK_GATE(tdmin_lb, AUDIO_CLK_GATE_EN0, 3);
> +struct clk_regmap aud_loopback =
> + AUD_PCLK_GATE(loopback, AUDIO_CLK_GATE_EN0, 4);
> +struct clk_regmap aud_tdmout_a =
> + AUD_PCLK_GATE(tdmout_a, AUDIO_CLK_GATE_EN0, 5);
> +struct clk_regmap aud_tdmout_b =
> + AUD_PCLK_GATE(tdmout_b, AUDIO_CLK_GATE_EN0, 6);
> +struct clk_regmap aud_frddr_a =
> + AUD_PCLK_GATE(frddr_a, AUDIO_CLK_GATE_EN0, 7);
> +struct clk_regmap aud_frddr_b =
> + AUD_PCLK_GATE(frddr_b, AUDIO_CLK_GATE_EN0, 8);
> +struct clk_regmap aud_toddr_a =
> + AUD_PCLK_GATE(toddr_a, AUDIO_CLK_GATE_EN0, 9);
> +struct clk_regmap aud_toddr_b =
> + AUD_PCLK_GATE(toddr_b, AUDIO_CLK_GATE_EN0, 10);
> +struct clk_regmap aud_spdifin =
> + AUD_PCLK_GATE(spdifin, AUDIO_CLK_GATE_EN0, 11);
> +struct clk_regmap aud_resample =
> + AUD_PCLK_GATE(resample, AUDIO_CLK_GATE_EN0, 12);
> +struct clk_regmap aud_eqdrc =
> + AUD_PCLK_GATE(eqdrc, AUDIO_CLK_GATE_EN0, 13);
> +struct clk_regmap aud_audiolocker =
> + AUD_PCLK_GATE(audiolocker, AUDIO_CLK_GATE_EN0, 14);
> +
> +struct clk_regmap aud2_ddr_arb =
> + AUD_PCLK_GATE(2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0);
> +struct clk_regmap aud2_pdm =
> + AUD_PCLK_GATE(2_pdm, AUDIO2_CLK_GATE_EN0, 1);
> +struct clk_regmap aud2_tdmin_vad =
> + AUD_PCLK_GATE(2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2);
> +struct clk_regmap aud2_toddr_vad =
> + AUD_PCLK_GATE(2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3);
> +struct clk_regmap aud2_vad =
> + AUD_PCLK_GATE(2_vad, AUDIO2_CLK_GATE_EN0, 4);
> +struct clk_regmap aud2_audiotop =
> + AUD_PCLK_GATE(2_audiotop, AUDIO2_CLK_GATE_EN0, 7);
> +
> +static const struct clk_parent_data a1_mst_pdata[] = {
> + { .fw_name = "dds_in" },
> + { .fw_name = "fclk_div2" },
> + { .fw_name = "fclk_div3" },
> + { .fw_name = "hifi_pll" },
> + { .fw_name = "xtal" },
> +};
> +
> +#define AUD_MST_MCLK_MUX(_name, _reg) \
> + AUD_MUX(_name##_sel, _reg, 0x7, 24, CLK_MUX_ROUND_CLOSEST, \
> + a1_mst_pdata, 0)
> +#define AUD_MST_MCLK_DIV(_name, _reg) \
> + AUD_DIV(_name##_div, _reg, 0, 16, CLK_DIVIDER_ROUND_CLOSEST, \
> + aud_##_name##_sel, CLK_SET_RATE_PARENT)
> +#define AUD_MST_MCLK_GATE(_name, _reg) \
> + AUD_GATE(_name, _reg, 31, aud_##_name##_div, \
> + CLK_SET_RATE_PARENT)
> +
> +struct clk_regmap aud_mst_a_mclk_mux =
> + AUD_MST_MCLK_MUX(mst_a_mclk, AUDIO_MCLK_A_CTRL);
> +struct clk_regmap aud_mst_a_mclk_div =
> + AUD_MST_MCLK_DIV(mst_a_mclk, AUDIO_MCLK_A_CTRL);
> +struct clk_regmap aud_mst_a_mclk =
> + AUD_MST_MCLK_GATE(mst_a_mclk, AUDIO_MCLK_A_CTRL);
> +
> +struct clk_regmap aud_mst_b_mclk_mux =
> + AUD_MST_MCLK_MUX(mst_b_mclk, AUDIO_MCLK_B_CTRL);
> +struct clk_regmap aud_mst_b_mclk_div =
> + AUD_MST_MCLK_DIV(mst_b_mclk, AUDIO_MCLK_B_CTRL);
> +struct clk_regmap aud_mst_b_mclk =
> + AUD_MST_MCLK_GATE(mst_b_mclk, AUDIO_MCLK_B_CTRL);
> +
> +struct clk_regmap aud_mst_c_mclk_mux =
> + AUD_MST_MCLK_MUX(mst_c_mclk, AUDIO_MCLK_C_CTRL);
> +struct clk_regmap aud_mst_c_mclk_div =
> + AUD_MST_MCLK_DIV(mst_c_mclk, AUDIO_MCLK_C_CTRL);
> +struct clk_regmap aud_mst_c_mclk =
> + AUD_MST_MCLK_GATE(mst_c_mclk, AUDIO_MCLK_C_CTRL);
> +
> +struct clk_regmap aud_mst_d_mclk_mux =
> + AUD_MST_MCLK_MUX(mst_d_mclk, AUDIO_MCLK_D_CTRL);
> +struct clk_regmap aud_mst_d_mclk_div =
> + AUD_MST_MCLK_DIV(mst_d_mclk, AUDIO_MCLK_D_CTRL);
> +struct clk_regmap aud_mst_d_mclk =
> + AUD_MST_MCLK_GATE(mst_d_mclk, AUDIO_MCLK_D_CTRL);
> +
> +struct clk_regmap aud_spdifin_clk_mux =
> + AUD_MST_MCLK_MUX(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
> +struct clk_regmap aud_spdifin_clk_div =
> + AUD_MST_MCLK_DIV(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
> +struct clk_regmap aud_spdifin_clk =
> + AUD_MST_MCLK_GATE(spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
> +
> +struct clk_regmap aud_eqdrc_clk_mux =
> + AUD_MST_MCLK_MUX(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
> +struct clk_regmap aud_eqdrc_clk_div =
> + AUD_MST_MCLK_DIV(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
> +struct clk_regmap aud_eqdrc_clk =
> + AUD_MST_MCLK_GATE(eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
> +
> +struct clk_regmap aud_resample_clk_mux =
> + AUD_MUX(resample_clk_sel, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
> + CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_resample_clk_div =
> + AUD_DIV(resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
> + CLK_DIVIDER_ROUND_CLOSEST, resample_clk_sel, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_resample_clk =
> + AUD_GATE(resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
> + resample_clk_div, CLK_SET_RATE_PARENT);
> +
> +struct clk_regmap aud_locker_in_clk_mux =
> + AUD_MUX(locker_in_clk_sel, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
> + CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_locker_in_clk_div =
> + AUD_DIV(locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
> + CLK_DIVIDER_ROUND_CLOSEST, locker_in_clk_sel, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_locker_in_clk =
> + AUD_GATE(locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
> + locker_in_clk_div, CLK_SET_RATE_PARENT);
> +
> +struct clk_regmap aud_locker_out_clk_mux =
> + AUD_MUX(locker_out_clk_sel, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
> + CLK_MUX_ROUND_CLOSEST, a1_mst_pdata, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_locker_out_clk_div =
> + AUD_DIV(locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
> + CLK_DIVIDER_ROUND_CLOSEST, locker_out_clk_sel, CLK_SET_RATE_PARENT);
> +struct clk_regmap aud_locker_out_clk =
> + AUD_GATE(locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
> + locker_out_clk_div, CLK_SET_RATE_PARENT);
> +
> +struct clk_regmap aud2_vad_mclk_mux =
> + AUD_MST_MCLK_MUX(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
> +struct clk_regmap aud2_vad_mclk_div =
> + AUD_MST_MCLK_DIV(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
> +struct clk_regmap aud2_vad_mclk =
> + AUD_MST_MCLK_GATE(2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
> +
> +struct clk_regmap aud2_vad_clk_mux =
> + AUD_MST_MCLK_MUX(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
> +struct clk_regmap aud2_vad_clk_div =
> + AUD_MST_MCLK_DIV(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
> +struct clk_regmap aud2_vad_clk =
> + AUD_MST_MCLK_GATE(2_vad_clk, AUDIO2_CLK_VAD_CTRL);
> +
> +struct clk_regmap aud2_pdm_dclk_mux =
> + AUD_MST_MCLK_MUX(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
> +struct clk_regmap aud2_pdm_dclk_div =
> + AUD_MST_MCLK_DIV(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
> +struct clk_regmap aud2_pdm_dclk =
> + AUD_MST_MCLK_GATE(2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
> +
> +struct clk_regmap aud2_pdm_sysclk_mux =
> + AUD_MST_MCLK_MUX(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
> +struct clk_regmap aud2_pdm_sysclk_div =
> + AUD_MST_MCLK_DIV(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
> +struct clk_regmap aud2_pdm_sysclk =
> + AUD_MST_MCLK_GATE(2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
> +
> +#define AUD_MST_SCLK_PRE_EN(_name, _reg, _pname) \
> + AUD_GATE(_name##_pre_en, _reg, 31, \
> + aud_##_pname, 0)
> +#define AUD_MST_SCLK_DIV(_name, _reg) \
> + AUD_SCLK_DIV(_name##_div, _reg, 20, 10, 0, 0, \
> + aud_##_name##_pre_en, CLK_SET_RATE_PARENT)
> +#define AUD_MST_SCLK_POST_EN(_name, _reg) \
> + AUD_GATE(_name##_post_en, _reg, 30, \
> + aud_##_name##_div, CLK_SET_RATE_PARENT)
> +#define AUD_MST_SCLK(_name, _reg) \
> + AUD_TRIPHASE(_name, _reg, 1, 0, 2, 4, \
> + aud_##_name##_post_en, CLK_SET_RATE_PARENT)
> +
> +struct clk_regmap aud_mst_a_sclk_pre_en =
> + AUD_MST_SCLK_PRE_EN(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, mst_a_mclk);
> +struct clk_regmap aud_mst_a_sclk_div =
> + AUD_MST_SCLK_DIV(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0);
> +struct clk_regmap aud_mst_a_sclk_post_en =
> + AUD_MST_SCLK_POST_EN(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0);
> +struct clk_regmap aud_mst_a_sclk =
> + AUD_MST_SCLK(mst_a_sclk, AUDIO_MST_A_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_b_sclk_pre_en =
> + AUD_MST_SCLK_PRE_EN(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, mst_b_mclk);
> +struct clk_regmap aud_mst_b_sclk_div =
> + AUD_MST_SCLK_DIV(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0);
> +struct clk_regmap aud_mst_b_sclk_post_en =
> + AUD_MST_SCLK_POST_EN(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0);
> +struct clk_regmap aud_mst_b_sclk =
> + AUD_MST_SCLK(mst_b_sclk, AUDIO_MST_B_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_c_sclk_pre_en =
> + AUD_MST_SCLK_PRE_EN(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, mst_c_mclk);
> +struct clk_regmap aud_mst_c_sclk_div =
> + AUD_MST_SCLK_DIV(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0);
> +struct clk_regmap aud_mst_c_sclk_post_en =
> + AUD_MST_SCLK_POST_EN(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0);
> +struct clk_regmap aud_mst_c_sclk =
> + AUD_MST_SCLK(mst_c_sclk, AUDIO_MST_C_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_d_sclk_pre_en =
> + AUD_MST_SCLK_PRE_EN(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, mst_d_mclk);
> +struct clk_regmap aud_mst_d_sclk_div =
> + AUD_MST_SCLK_DIV(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0);
> +struct clk_regmap aud_mst_d_sclk_post_en =
> + AUD_MST_SCLK_POST_EN(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0);
> +struct clk_regmap aud_mst_d_sclk =
> + AUD_MST_SCLK(mst_d_sclk, AUDIO_MST_D_SCLK_CTRL1);
> +
> +#define AUD_MST_LRCLK_DIV(_name, _reg, _pname) \
> + AUD_SCLK_DIV(_name##_div, _reg, 0, 10, 10, 10, \
> + aud_##_pname, 0)
> +#define AUD_MST_LRCLK(_name, _reg) \
> + AUD_TRIPHASE(_name, _reg, 1, 1, 3, 5, \
> + aud_##_name##_div, CLK_SET_RATE_PARENT)
> +
> +struct clk_regmap aud_mst_a_lrclk_div =
> + AUD_MST_LRCLK_DIV(mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, mst_a_sclk_post_en);
> +struct clk_regmap aud_mst_a_lrclk =
> + AUD_MST_LRCLK(mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_b_lrclk_div =
> + AUD_MST_LRCLK_DIV(mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, mst_b_sclk_post_en);
> +struct clk_regmap aud_mst_b_lrclk =
> + AUD_MST_LRCLK(mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_c_lrclk_div =
> + AUD_MST_LRCLK_DIV(mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, mst_c_sclk_post_en);
> +struct clk_regmap aud_mst_c_lrclk =
> + AUD_MST_LRCLK(mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL1);
> +
> +struct clk_regmap aud_mst_d_lrclk_div =
> + AUD_MST_LRCLK_DIV(mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, mst_d_sclk_post_en);
> +struct clk_regmap aud_mst_d_lrclk =
> + AUD_MST_LRCLK(mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL1);
> +
> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
> + { .hw = &aud_mst_a_sclk.hw, .index = -1 },
> + { .hw = &aud_mst_b_sclk.hw, .index = -1 },
> + { .hw = &aud_mst_c_sclk.hw, .index = -1 },
> + { .hw = &aud_mst_d_sclk.hw, .index = -1 },
> + { .fw_name = "slv_sclk0" },
> + { .fw_name = "slv_sclk1" },
> + { .fw_name = "slv_sclk2" },
> + { .fw_name = "slv_sclk3" },
> + { .fw_name = "slv_sclk4" },
> + { .fw_name = "slv_sclk5" },
> + { .fw_name = "slv_sclk6" },
> + { .fw_name = "slv_sclk7" },
> + { .fw_name = "slv_sclk8" },
> + { .fw_name = "slv_sclk9" },
> +};
> +
> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
> + { .hw = &aud_mst_a_lrclk.hw, .index = -1 },
> + { .hw = &aud_mst_b_lrclk.hw, .index = -1 },
> + { .hw = &aud_mst_c_lrclk.hw, .index = -1 },
> + { .hw = &aud_mst_d_lrclk.hw, .index = -1 },
> + { .fw_name = "slv_lrclk0" },
> + { .fw_name = "slv_lrclk1" },
> + { .fw_name = "slv_lrclk2" },
> + { .fw_name = "slv_lrclk3" },
> + { .fw_name = "slv_lrclk4" },
> + { .fw_name = "slv_lrclk5" },
> + { .fw_name = "slv_lrclk6" },
> + { .fw_name = "slv_lrclk7" },
> + { .fw_name = "slv_lrclk8" },
> + { .fw_name = "slv_lrclk9" },
> +};
> +
> +#define AUD_TDM_SCLK_MUX(_name, _reg) \
> + AUD_MUX(_name##_sel, _reg, 0xf, 24, \
> + CLK_MUX_ROUND_CLOSEST, a1_mst_sclk_pdata, 0)
> +#define AUD_TDM_SCLK_PRE_EN(_name, _reg) \
> + AUD_GATE(_name##_pre_en, _reg, 31, \
> + aud_##_name##_sel, CLK_SET_RATE_PARENT)
> +#define AUD_TDM_SCLK_POST_EN(_name, _reg) \
> + AUD_GATE(_name##_post_en, _reg, 30, \
> + aud_##_name##_pre_en, CLK_SET_RATE_PARENT)
> +#define AUD_TDM_SCLK_WS(_name, _reg) \
> + AUD_SCLK_WS(_name, _reg, 1, 29, 28, \
> + aud_##_name##_post_en, \
> + CLK_DUTY_CYCLE_PARENT | CLK_SET_RATE_PARENT)
> +
> +#define AUD_TDM_LRLCK(_name, _reg) \
> + AUD_MUX(_name, _reg, 0xf, 20, \
> + CLK_MUX_ROUND_CLOSEST, a1_mst_lrclk_pdata, \
> + CLK_SET_RATE_PARENT)
> +
> +struct clk_regmap aud_tdmin_a_sclk_mux =
> + AUD_TDM_SCLK_MUX(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
> +struct clk_regmap aud_tdmin_a_sclk_pre_en =
> + AUD_TDM_SCLK_PRE_EN(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
> +struct clk_regmap aud_tdmin_a_sclk_post_en =
> + AUD_TDM_SCLK_POST_EN(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
> +struct clk_regmap aud_tdmin_a_sclk =
> + AUD_TDM_SCLK_WS(tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
> +struct clk_regmap aud_tdmin_a_lrclk =
> + AUD_TDM_LRLCK(tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
> +
> +struct clk_regmap aud_tdmin_b_sclk_mux =
> + AUD_TDM_SCLK_MUX(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
> +struct clk_regmap aud_tdmin_b_sclk_pre_en =
> + AUD_TDM_SCLK_PRE_EN(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
> +struct clk_regmap aud_tdmin_b_sclk_post_en =
> + AUD_TDM_SCLK_POST_EN(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
> +struct clk_regmap aud_tdmin_b_sclk =
> + AUD_TDM_SCLK_WS(tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
> +struct clk_regmap aud_tdmin_b_lrclk =
> + AUD_TDM_LRLCK(tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
> +
> +struct clk_regmap aud_tdmin_lb_sclk_mux =
> + AUD_TDM_SCLK_MUX(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
> +struct clk_regmap aud_tdmin_lb_sclk_pre_en =
> + AUD_TDM_SCLK_PRE_EN(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
> +struct clk_regmap aud_tdmin_lb_sclk_post_en =
> + AUD_TDM_SCLK_POST_EN(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
> +struct clk_regmap aud_tdmin_lb_sclk =
> + AUD_TDM_SCLK_WS(tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
> +struct clk_regmap aud_tdmin_lb_lrclk =
> + AUD_TDM_LRLCK(tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
> +
> +struct clk_regmap aud_tdmout_a_sclk_mux =
> + AUD_TDM_SCLK_MUX(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
> +struct clk_regmap aud_tdmout_a_sclk_pre_en =
> + AUD_TDM_SCLK_PRE_EN(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
> +struct clk_regmap aud_tdmout_a_sclk_post_en =
> + AUD_TDM_SCLK_POST_EN(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
> +struct clk_regmap aud_tdmout_a_sclk =
> + AUD_TDM_SCLK_WS(tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
> +struct clk_regmap aud_tdmout_a_lrclk =
> + AUD_TDM_LRLCK(tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
> +
> +struct clk_regmap aud_tdmout_b_sclk_mux =
> + AUD_TDM_SCLK_MUX(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
> +struct clk_regmap aud_tdmout_b_sclk_pre_en =
> + AUD_TDM_SCLK_PRE_EN(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
> +struct clk_regmap aud_tdmout_b_sclk_post_en =
> + AUD_TDM_SCLK_POST_EN(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
> +struct clk_regmap aud_tdmout_b_sclk =
> + AUD_TDM_SCLK_WS(tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
> +struct clk_regmap aud_tdmout_b_lrclk =
> + AUD_TDM_LRLCK(tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
> +
> +static struct clk_hw *a1_audio_hw_clks[] = {
> + [AUD_CLKID_DDR_ARB] = &aud_ddr_arb.hw,
> + [AUD_CLKID_TDMIN_A] = &aud_tdmin_a.hw,
> + [AUD_CLKID_TDMIN_B] = &aud_tdmin_b.hw,
> + [AUD_CLKID_TDMIN_LB] = &aud_tdmin_lb.hw,
> + [AUD_CLKID_LOOPBACK] = &aud_loopback.hw,
> + [AUD_CLKID_TDMOUT_A] = &aud_tdmout_a.hw,
> + [AUD_CLKID_TDMOUT_B] = &aud_tdmout_b.hw,
> + [AUD_CLKID_FRDDR_A] = &aud_frddr_a.hw,
> + [AUD_CLKID_FRDDR_B] = &aud_frddr_b.hw,
> + [AUD_CLKID_TODDR_A] = &aud_toddr_a.hw,
> + [AUD_CLKID_TODDR_B] = &aud_toddr_b.hw,
> + [AUD_CLKID_SPDIFIN] = &aud_spdifin.hw,
> + [AUD_CLKID_RESAMPLE] = &aud_resample.hw,
> + [AUD_CLKID_EQDRC] = &aud_eqdrc.hw,
> + [AUD_CLKID_LOCKER] = &aud_audiolocker.hw,
> + [AUD_CLKID_MST_A_MCLK_SEL] = &aud_mst_a_mclk_mux.hw,
> + [AUD_CLKID_MST_A_MCLK_DIV] = &aud_mst_a_mclk_div.hw,
> + [AUD_CLKID_MST_A_MCLK] = &aud_mst_a_mclk.hw,
> + [AUD_CLKID_MST_B_MCLK_SEL] = &aud_mst_b_mclk_mux.hw,
> + [AUD_CLKID_MST_B_MCLK_DIV] = &aud_mst_b_mclk_div.hw,
> + [AUD_CLKID_MST_B_MCLK] = &aud_mst_b_mclk.hw,
> + [AUD_CLKID_MST_C_MCLK_SEL] = &aud_mst_c_mclk_mux.hw,
> + [AUD_CLKID_MST_C_MCLK_DIV] = &aud_mst_c_mclk_div.hw,
> + [AUD_CLKID_MST_C_MCLK] = &aud_mst_c_mclk.hw,
> + [AUD_CLKID_MST_D_MCLK_SEL] = &aud_mst_d_mclk_mux.hw,
> + [AUD_CLKID_MST_D_MCLK_DIV] = &aud_mst_d_mclk_div.hw,
> + [AUD_CLKID_MST_D_MCLK] = &aud_mst_d_mclk.hw,
> + [AUD_CLKID_RESAMPLE_CLK_SEL] = &aud_resample_clk_mux.hw,
> + [AUD_CLKID_RESAMPLE_CLK_DIV] = &aud_resample_clk_div.hw,
> + [AUD_CLKID_RESAMPLE_CLK] = &aud_resample_clk.hw,
> + [AUD_CLKID_LOCKER_IN_CLK_SEL] = &aud_locker_in_clk_mux.hw,
> + [AUD_CLKID_LOCKER_IN_CLK_DIV] = &aud_locker_in_clk_div.hw,
> + [AUD_CLKID_LOCKER_IN_CLK] = &aud_locker_in_clk.hw,
> + [AUD_CLKID_LOCKER_OUT_CLK_SEL] = &aud_locker_out_clk_mux.hw,
> + [AUD_CLKID_LOCKER_OUT_CLK_DIV] = &aud_locker_out_clk_div.hw,
> + [AUD_CLKID_LOCKER_OUT_CLK] = &aud_locker_out_clk.hw,
> + [AUD_CLKID_SPDIFIN_CLK_SEL] = &aud_spdifin_clk_mux.hw,
> + [AUD_CLKID_SPDIFIN_CLK_DIV] = &aud_spdifin_clk_div.hw,
> + [AUD_CLKID_SPDIFIN_CLK] = &aud_spdifin_clk.hw,
> + [AUD_CLKID_EQDRC_CLK_SEL] = &aud_eqdrc_clk_mux.hw,
> + [AUD_CLKID_EQDRC_CLK_DIV] = &aud_eqdrc_clk_div.hw,
> + [AUD_CLKID_EQDRC_CLK] = &aud_eqdrc_clk.hw,
> + [AUD_CLKID_MST_A_SCLK_PRE_EN] = &aud_mst_a_sclk_pre_en.hw,
> + [AUD_CLKID_MST_A_SCLK_DIV] = &aud_mst_a_sclk_div.hw,
> + [AUD_CLKID_MST_A_SCLK_POST_EN] = &aud_mst_a_sclk_post_en.hw,
> + [AUD_CLKID_MST_A_SCLK] = &aud_mst_a_sclk.hw,
> + [AUD_CLKID_MST_B_SCLK_PRE_EN] = &aud_mst_b_sclk_pre_en.hw,
> + [AUD_CLKID_MST_B_SCLK_DIV] = &aud_mst_b_sclk_div.hw,
> + [AUD_CLKID_MST_B_SCLK_POST_EN] = &aud_mst_b_sclk_post_en.hw,
> + [AUD_CLKID_MST_B_SCLK] = &aud_mst_b_sclk.hw,
> + [AUD_CLKID_MST_C_SCLK_PRE_EN] = &aud_mst_c_sclk_pre_en.hw,
> + [AUD_CLKID_MST_C_SCLK_DIV] = &aud_mst_c_sclk_div.hw,
> + [AUD_CLKID_MST_C_SCLK_POST_EN] = &aud_mst_c_sclk_post_en.hw,
> + [AUD_CLKID_MST_C_SCLK] = &aud_mst_c_sclk.hw,
> + [AUD_CLKID_MST_D_SCLK_PRE_EN] = &aud_mst_d_sclk_pre_en.hw,
> + [AUD_CLKID_MST_D_SCLK_DIV] = &aud_mst_d_sclk_div.hw,
> + [AUD_CLKID_MST_D_SCLK_POST_EN] = &aud_mst_d_sclk_post_en.hw,
> + [AUD_CLKID_MST_D_SCLK] = &aud_mst_d_sclk.hw,
> + [AUD_CLKID_MST_A_LRCLK_DIV] = &aud_mst_a_lrclk_div.hw,
> + [AUD_CLKID_MST_A_LRCLK] = &aud_mst_a_lrclk.hw,
> + [AUD_CLKID_MST_B_LRCLK_DIV] = &aud_mst_b_lrclk_div.hw,
> + [AUD_CLKID_MST_B_LRCLK] = &aud_mst_b_lrclk.hw,
> + [AUD_CLKID_MST_C_LRCLK_DIV] = &aud_mst_c_lrclk_div.hw,
> + [AUD_CLKID_MST_C_LRCLK] = &aud_mst_c_lrclk.hw,
> + [AUD_CLKID_MST_D_LRCLK_DIV] = &aud_mst_d_lrclk_div.hw,
> + [AUD_CLKID_MST_D_LRCLK] = &aud_mst_d_lrclk.hw,
> + [AUD_CLKID_TDMIN_A_SCLK_SEL] = &aud_tdmin_a_sclk_mux.hw,
> + [AUD_CLKID_TDMIN_A_SCLK_PRE_EN] = &aud_tdmin_a_sclk_pre_en.hw,
> + [AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &aud_tdmin_a_sclk_post_en.hw,
> + [AUD_CLKID_TDMIN_A_SCLK] = &aud_tdmin_a_sclk.hw,
> + [AUD_CLKID_TDMIN_A_LRCLK] = &aud_tdmin_a_lrclk.hw,
> + [AUD_CLKID_TDMIN_B_SCLK_SEL] = &aud_tdmin_b_sclk_mux.hw,
> + [AUD_CLKID_TDMIN_B_SCLK_PRE_EN] = &aud_tdmin_b_sclk_pre_en.hw,
> + [AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &aud_tdmin_b_sclk_post_en.hw,
> + [AUD_CLKID_TDMIN_B_SCLK] = &aud_tdmin_b_sclk.hw,
> + [AUD_CLKID_TDMIN_B_LRCLK] = &aud_tdmin_b_lrclk.hw,
> + [AUD_CLKID_TDMIN_LB_SCLK_SEL] = &aud_tdmin_lb_sclk_mux.hw,
> + [AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &aud_tdmin_lb_sclk_pre_en.hw,
> + [AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &aud_tdmin_lb_sclk_post_en.hw,
> + [AUD_CLKID_TDMIN_LB_SCLK] = &aud_tdmin_lb_sclk.hw,
> + [AUD_CLKID_TDMIN_LB_LRCLK] = &aud_tdmin_lb_lrclk.hw,
> + [AUD_CLKID_TDMOUT_A_SCLK_SEL] = &aud_tdmout_a_sclk_mux.hw,
> + [AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &aud_tdmout_a_sclk_pre_en.hw,
> + [AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &aud_tdmout_a_sclk_post_en.hw,
> + [AUD_CLKID_TDMOUT_A_SCLK] = &aud_tdmout_a_sclk.hw,
> + [AUD_CLKID_TDMOUT_A_LRCLK] = &aud_tdmout_a_lrclk.hw,
> + [AUD_CLKID_TDMOUT_B_SCLK_SEL] = &aud_tdmout_b_sclk_mux.hw,
> + [AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &aud_tdmout_b_sclk_pre_en.hw,
> + [AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &aud_tdmout_b_sclk_post_en.hw,
> + [AUD_CLKID_TDMOUT_B_SCLK] = &aud_tdmout_b_sclk.hw,
> + [AUD_CLKID_TDMOUT_B_LRCLK] = &aud_tdmout_b_lrclk.hw,
> +};
> +
> +static struct clk_hw *a1_audio2_hw_clks[] = {
> + [AUD2_CLKID_DDR_ARB] = &aud2_ddr_arb.hw,
> + [AUD2_CLKID_PDM] = &aud2_pdm.hw,
> + [AUD2_CLKID_TDMIN_VAD] = &aud2_tdmin_vad.hw,
> + [AUD2_CLKID_TODDR_VAD] = &aud2_toddr_vad.hw,
> + [AUD2_CLKID_VAD] = &aud2_vad.hw,
> + [AUD2_CLKID_AUDIOTOP] = &aud2_audiotop.hw,
> + [AUD2_CLKID_VAD_MCLK_SEL] = &aud2_vad_mclk_mux.hw,
> + [AUD2_CLKID_VAD_MCLK_DIV] = &aud2_vad_mclk_div.hw,
> + [AUD2_CLKID_VAD_MCLK] = &aud2_vad_mclk.hw,
> + [AUD2_CLKID_VAD_CLK_SEL] = &aud2_vad_clk_mux.hw,
> + [AUD2_CLKID_VAD_CLK_DIV] = &aud2_vad_clk_div.hw,
> + [AUD2_CLKID_VAD_CLK] = &aud2_vad_clk.hw,
> + [AUD2_CLKID_PDM_DCLK_SEL] = &aud2_pdm_dclk_mux.hw,
> + [AUD2_CLKID_PDM_DCLK_DIV] = &aud2_pdm_dclk_div.hw,
> + [AUD2_CLKID_PDM_DCLK] = &aud2_pdm_dclk.hw,
> + [AUD2_CLKID_PDM_SYSCLK_SEL] = &aud2_pdm_sysclk_mux.hw,
> + [AUD2_CLKID_PDM_SYSCLK_DIV] = &aud2_pdm_sysclk_div.hw,
> + [AUD2_CLKID_PDM_SYSCLK] = &aud2_pdm_sysclk.hw,
> +};
I think you already got that comment but audio2 is a terrible name.
Given that the 2nd controller is mostly about VAD (PDM is there only to
feed it) and that it is just a name, I'd prefer something like a1_audio_vad.
> +
> +static int a1_register_clk(struct platform_device *pdev, struct regmap *map,
> + struct clk_hw *hw)
> +{
> + struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
> +
> + if (!hw)
> + return 0;
> +
> + clk->map = map;
> +
> + return devm_clk_hw_register(&pdev->dev, hw);
> +}
Why do you have to do that and cannot do it like the rest of the
modules ?
> +
> +struct a1_audio_data {
> + struct meson_clk_hw_data hw_clks;
> + int core_clkid;
> + const char *core_fwname;
> + unsigned int reset_offset;
> + unsigned int reset_num;
> +};
> +
> +static const struct regmap_config a1_audio_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +static int a1_audio_clkc_probe(struct platform_device *pdev)
> +{
> + const struct a1_audio_data *data;
> + struct regmap *map;
> + void __iomem *base;
> + struct clk *clk;
> + unsigned int i;
> + int ret;
> +
> + data = of_device_get_match_data(&pdev->dev);
> + if (!data)
> + return -EINVAL;
> +
> + if (data->core_fwname) {
That is really over-complicated for what it does ....
> + clk = devm_clk_get_enabled(&pdev->dev, data->core_fwname);
It does not make a lot of sense that one of the 2 controllers as 2nd
pclk.
You should consider using the AUD2_CLKID_AUDIOTOP as a parent of audio
gates instead. You would not have to that.
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> + }
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + map = devm_regmap_init_mmio(&pdev->dev, base, &a1_audio_regmap_cfg);
> + if (IS_ERR(map))
> + return PTR_ERR(map);
> +
> + clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> + if (IS_ERR(clk))
> + return PTR_ERR(clk);
> +
> + for (i = 0; i < data->hw_clks.num; i++) {
> + struct clk_hw *hw = data->hw_clks.hws[i];
> +
> + ret = a1_register_clk(pdev, map, hw);
> + if (ret)
> + return ret;
> + }
> +
> + ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
> + (void *)&data->hw_clks);
> + if (ret)
> + return ret;
> +
> + if (!data->reset_num)
> + return 0;
> +
> + return meson_audio_rstc_register(&pdev->dev, map, data->reset_offset,
> + data->reset_num);
> +}
Again this is over-complicated.
Just make an external function for the register, use it for axg, a1,
a1-vad, then make one file for each controller. That will keep the code
clean and readable.
keep it simple please.
> +
> +struct a1_audio_data a1_audio_data = {
> + .hw_clks = {
> + .hws = a1_audio_hw_clks,
> + .num = ARRAY_SIZE(a1_audio_hw_clks),
> + },
> + .core_fwname = "core",
> + .reset_offset = AUDIO_SW_RESET0,
> + .reset_num = 32,
> +};
> +
> +struct a1_audio_data a1_audio2_data = {
> + .hw_clks = {
> + .hws = a1_audio2_hw_clks,
> + .num = ARRAY_SIZE(a1_audio2_hw_clks),
> + },
> +};
> +
> +static const struct of_device_id a1_audio_clkc_match_table[] = {
> + {
> + .compatible = "amlogic,a1-audio-clkc",
> + .data = &a1_audio_data,
> + },
> + {
> + .compatible = "amlogic,a1-audio2-clkc",
> + .data = &a1_audio2_data,
> + },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
> +
> +static struct platform_driver a1_audio_clkc_driver = {
> + .probe = a1_audio_clkc_probe,
> + .driver = {
> + .name = "a1-audio-clkc",
> + .of_match_table = a1_audio_clkc_match_table,
> + },
> +};
> +module_platform_driver(a1_audio_clkc_driver);
> +
> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
> new file mode 100644
> index 000000000000..9ea9da21ff9b
> --- /dev/null
> +++ b/drivers/clk/meson/a1-audio.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
> + *
> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
> + */
> +
> +#ifndef __A1_AUDIO_H
> +#define __A1_AUDIO_H
> +
> +#define AUDIO_CLK_GATE_EN0 0x000
> +#define AUDIO_MCLK_A_CTRL 0x008
> +#define AUDIO_MCLK_B_CTRL 0x00c
> +#define AUDIO_MCLK_C_CTRL 0x010
> +#define AUDIO_MCLK_D_CTRL 0x014
> +#define AUDIO_MCLK_E_CTRL 0x018
> +#define AUDIO_MCLK_F_CTRL 0x01c
> +#define AUDIO_SW_RESET0 0x028
> +#define AUDIO_MST_A_SCLK_CTRL0 0x040
> +#define AUDIO_MST_A_SCLK_CTRL1 0x044
> +#define AUDIO_MST_B_SCLK_CTRL0 0x048
> +#define AUDIO_MST_B_SCLK_CTRL1 0x04c
> +#define AUDIO_MST_C_SCLK_CTRL0 0x050
> +#define AUDIO_MST_C_SCLK_CTRL1 0x054
> +#define AUDIO_MST_D_SCLK_CTRL0 0x058
> +#define AUDIO_MST_D_SCLK_CTRL1 0x05c
> +#define AUDIO_CLK_TDMIN_A_CTRL 0x080
> +#define AUDIO_CLK_TDMIN_B_CTRL 0x084
> +#define AUDIO_CLK_TDMIN_LB_CTRL 0x08c
> +#define AUDIO_CLK_TDMOUT_A_CTRL 0x090
> +#define AUDIO_CLK_TDMOUT_B_CTRL 0x094
> +#define AUDIO_CLK_SPDIFIN_CTRL 0x09c
> +#define AUDIO_CLK_RESAMPLE_CTRL 0x0a4
> +#define AUDIO_CLK_LOCKER_CTRL 0x0a8
> +#define AUDIO_CLK_EQDRC_CTRL 0x0c0
> +
> +#define AUDIO2_CLK_GATE_EN0 0x00c
> +#define AUDIO2_MCLK_VAD_CTRL 0x040
> +#define AUDIO2_CLK_VAD_CTRL 0x044
> +#define AUDIO2_CLK_PDMIN_CTRL0 0x058
> +#define AUDIO2_CLK_PDMIN_CTRL1 0x05c
Same remark - header is useless.
> +
> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
> +
> +#endif /* __A1_AUDIO_H */
--
Jerome
^ permalink raw reply
* Re: [PATCH v8 1/3] input: pm8xxx-vibrator: refactor to support new SPMI vibrator
From: Konrad Dybcio @ 2024-04-02 15:21 UTC (permalink / raw)
To: quic_fenglinw, kernel, Andy Gross, Bjorn Andersson,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree
In-Reply-To: <20240401-pm8xxx-vibrator-new-design-v8-1-6f2b8b03b4c7@quicinc.com>
On 1.04.2024 10:38 AM, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>
> Currently, vibrator control register addresses are hard coded,
> including the base address and offsets, it's not flexible to
> support new SPMI vibrator module which is usually included in
> different PMICs with different base address. Refactor it by using
> the base address defined in devicetree.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
[...]
> if (regs->enable_mask)
> - rc = regmap_update_bits(vib->regmap, regs->enable_addr,
> + rc = regmap_update_bits(vib->regmap, vib->enable_addr,
> regs->enable_mask, on ? ~0 : 0);
The idiomatic way across the kernel seems to be writing the mask value
instead of ~0 (which also saves like 2 cpu instructions)
Not sure about how ssbi addressing works, but except for that lgtm
Konrad
^ permalink raw reply
* [PATCH] arm64: dts: ti: k3-j722s: Disable ethernet ports by default
From: Michael Walle @ 2024-04-02 15:18 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Michael Walle
Device tree best practice is to disable any external interface in the
dtsi and just enable them if needed in the device tree. Thus, disable
both ethernet ports by default and just enable the one used by the EVM
in its device tree.
There is no functional change.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
This should also be true for all the other SoCs. But I don't wanted to
touch all the (older) device trees. j722s is pretty new, so there we
should get it right.
---
arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 5 +----
arch/arm64/boot/dts/ti/k3-j722s.dtsi | 8 ++++++++
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
index d045dc7dde0c..afe7f68e6a4b 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
@@ -224,14 +224,11 @@ cpsw3g_phy0: ethernet-phy@0 {
};
&cpsw_port1 {
+ status = "okay";
phy-mode = "rgmii-rxid";
phy-handle = <&cpsw3g_phy0>;
};
-&cpsw_port2 {
- status = "disabled";
-};
-
&main_gpio1 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/ti/k3-j722s.dtsi b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
index c75744edb143..d0451e6e7496 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j722s.dtsi
@@ -83,6 +83,14 @@ &inta_main_dmss {
ti,interrupt-ranges = <7 71 21>;
};
+&cpsw_port1 {
+ status = "disabled";
+};
+
+&cpsw_port2 {
+ status = "disabled";
+};
+
&oc_sram {
reg = <0x00 0x70000000 0x00 0x40000>;
ranges = <0x00 0x00 0x70000000 0x40000>;
--
2.39.2
^ permalink raw reply related
* [PATCH] arm64: dts: ti: k3-j722s-evm: Enable eMMC support
From: Michael Walle @ 2024-04-02 15:16 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, devicetree, linux-kernel, Michael Walle
The J722S EVM has an on-board eMMC. Enable the SDHC interface for it.
There is no pinmuxing required because the interface has dedicated pins.
Signed-off-by: Michael Walle <mwalle@kernel.org>
---
arch/arm64/boot/dts/ti/k3-j722s-evm.dts | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
index cee3a8661d5e..d045dc7dde0c 100644
--- a/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j722s-evm.dts
@@ -369,6 +369,13 @@ partition@3fc0000 {
};
+&sdhci0 {
+ status = "okay";
+ ti,driver-strength-ohm = <50>;
+ disable-wp;
+ bootph-all;
+};
+
&sdhci1 {
/* SD/MMC */
vmmc-supply = <&vdd_mmc1>;
--
2.39.2
^ permalink raw reply related
* Re: [PATCH 1/7] arm64: dts: imx8-ss-lsio: fix pwm lpcg indices
From: Frank Li @ 2024-04-02 15:09 UTC (permalink / raw)
To: Fabio Estevam
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
Marcel Ziswiler, Philippe Schenker, Max Krummenacher,
Alexander Stein, Joakim Zhang, devicetree, linux-arm-kernel,
linux-kernel, stable
In-Reply-To: <CAOMZO5AJrQ5jyV4A-tvX93-R0_nEWpEO9YY3f5DpeXaAFO4cSA@mail.gmail.com>
On Mon, Apr 01, 2024 at 08:04:56PM -0300, Fabio Estevam wrote:
> On Mon, Apr 1, 2024 at 7:25 PM Frank Li <Frank.Li@nxp.com> wrote:
> >
> > lpcg's arg0 should use clock indices instead of index.
> >
> > pwm0_lpcg: clock-controller@5d400000 {
> > ... // Col1 Col2
> > clocks = <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>, // 0 0
> > <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>, // 1 1
> > <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>, // 2 4
> > <&lsio_bus_clk>, // 3 5
> > <&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>; // 4 6
> > clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_1>,
> > <IMX_LPCG_CLK_4>, <IMX_LPCG_CLK_5>,
> > <IMX_LPCG_CLK_6>;
> > };
> >
> > Col1: index, which exited dts try to get.
>
> I cannot understand this sentence, sorry.
This base on downstream dts code. Downstream code use index in 'Col1' to
get clock.
For example:
<&pwm0_lpcg 3> means &lsio_bus_clk.
When someone do upstream, miss understand or omit upsteam lpcg driver's
difference between downstream and upstream version. And it also work even
index is wrong.
I realize this problem when I try to enable audio device for qm. The
difference cause audio can't work. The grep lpcg [0-9] to find this
problem.
>
> > Col2: actual index in lpcg driver.
>
> You should not describe DT in terms of Linux driver.
It just descript the actual hehavior in current drivers to explain why it
can work even arg0 is wrong.
for example: <&pwm0_lpcg 4>, developer intent to get
<&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>; // 4 6
but lpcg driver device device arg0 (4) by 4, get 1. So below clock return
<&clk IMX_SC_R_PWM_0 IMX_SC_PM_CLK_PER>, // 1 1
Both it is IMX_SC_PM_CLK_PER, so pwm works luckly.
But correct code should be <&pwm0_lpcg IMX_LPCG_CLK_6>.
Frank
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox