* [PATCH v2,5/5] media: mediatek: vcodec: fix incorrect sizeimage for 10bit bitstream
From: Yunfei Dong @ 2024-04-09 6:44 UTC (permalink / raw)
To: Nícolas F . R . A . Prado, Nicolas Dufresne, Hans Verkuil,
AngeloGioacchino Del Regno, Benjamin Gaignard, Nathan Hebert,
Sebastian Fricke
Cc: Hsin-Yi Wang, Fritz Koenig, Daniel Vetter, Steve Cho, Yunfei Dong,
linux-media, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Project_Global_Chrome_Upstream_Group
In-Reply-To: <20240409064431.16909-1-yunfei.dong@mediatek.com>
The sizeimage of each plane is calculated the same way for 8bit and
10bit bitstream. Using v4l2 common interface v4l2_fill_pixfmt_mp to
separate.
Fixes: 9d86be9bda6c ("media: mediatek: vcodec: Add driver to support 10bit")
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 28 ++++++-------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index 9107707de6c4..fbfba69682ea 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -262,40 +262,28 @@ static int vidioc_try_fmt(struct mtk_vcodec_dec_ctx *ctx, struct v4l2_format *f,
int tmp_w, tmp_h;
/*
- * Find next closer width align 64, height align 64, size align
- * 64 rectangle
+ * Find next closer width align 64, heign align 64, size align 64 rectangle
* Note: This only get default value, the real HW needed value
* only available when ctx in MTK_STATE_HEADER state
*/
tmp_w = pix_fmt_mp->width;
tmp_h = pix_fmt_mp->height;
+
v4l_bound_align_image(&pix_fmt_mp->width, MTK_VDEC_MIN_W, frmsize->max_width, 6,
&pix_fmt_mp->height, MTK_VDEC_MIN_H, frmsize->max_height, 6,
9);
- if (pix_fmt_mp->width < tmp_w &&
- (pix_fmt_mp->width + 64) <= frmsize->max_width)
+ if (pix_fmt_mp->width < tmp_w && (pix_fmt_mp->width + 64) <= frmsize->max_width)
pix_fmt_mp->width += 64;
- if (pix_fmt_mp->height < tmp_h &&
- (pix_fmt_mp->height + 64) <= frmsize->max_height)
+ if (pix_fmt_mp->height < tmp_h && (pix_fmt_mp->height + 64) <= frmsize->max_height)
pix_fmt_mp->height += 64;
+ v4l2_fill_pixfmt_mp(pix_fmt_mp, fmt->fourcc, pix_fmt_mp->width, pix_fmt_mp->height);
mtk_v4l2_vdec_dbg(0, ctx,
- "before resize wxh=%dx%d, after resize wxh=%dx%d, sizeimage=%d",
+ "before resize:%dx%d, after resize:%dx%d, sizeimage=0x%x_0x%x",
tmp_w, tmp_h, pix_fmt_mp->width, pix_fmt_mp->height,
- pix_fmt_mp->width * pix_fmt_mp->height);
-
- pix_fmt_mp->num_planes = fmt->num_planes;
- pix_fmt_mp->plane_fmt[0].sizeimage =
- pix_fmt_mp->width * pix_fmt_mp->height;
- pix_fmt_mp->plane_fmt[0].bytesperline = pix_fmt_mp->width;
-
- if (pix_fmt_mp->num_planes == 2) {
- pix_fmt_mp->plane_fmt[1].sizeimage =
- (pix_fmt_mp->width * pix_fmt_mp->height) / 2;
- pix_fmt_mp->plane_fmt[1].bytesperline =
- pix_fmt_mp->width;
- }
+ pix_fmt_mp->plane_fmt[0].sizeimage,
+ pix_fmt_mp->plane_fmt[1].sizeimage);
}
pix_fmt_mp->flags = 0;
--
2.18.0
^ permalink raw reply related
* [PATCH] riscv: dts: sophgo: add initial Milk-V Duo S board device tree
From: michael.opdenacker @ 2024-04-09 6:45 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen Wang,
Inochi Amaoto, Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: Michael Opdenacker, devicetree, linux-riscv, linux-kernel
From: Michael Opdenacker <michael.opdenacker@bootlin.com>
This adds initial support for the Milk-V Duo S board
(https://milkv.io/duo-s), enabling the serial port and
read-only SD card support, allowing to boot Linux to the
command line.
Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
---
Tested with linux-next as of Apr 9, 2024,
using the risc-v "defconfig" configuration.
---
arch/riscv/boot/dts/sophgo/Makefile | 1 +
.../boot/dts/sophgo/sg2000-milkv-duos.dts | 38 +++++++++++++++++++
2 files changed, 39 insertions(+)
create mode 100644 arch/riscv/boot/dts/sophgo/sg2000-milkv-duos.dts
diff --git a/arch/riscv/boot/dts/sophgo/Makefile b/arch/riscv/boot/dts/sophgo/Makefile
index 57ad82a61ea6..e008acb5240f 100644
--- a/arch/riscv/boot/dts/sophgo/Makefile
+++ b/arch/riscv/boot/dts/sophgo/Makefile
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_SOPHGO) += cv1800b-milkv-duo.dtb
dtb-$(CONFIG_ARCH_SOPHGO) += cv1812h-huashan-pi.dtb
+dtb-$(CONFIG_ARCH_SOPHGO) += sg2000-milkv-duos.dtb
dtb-$(CONFIG_ARCH_SOPHGO) += sg2042-milkv-pioneer.dtb
diff --git a/arch/riscv/boot/dts/sophgo/sg2000-milkv-duos.dts b/arch/riscv/boot/dts/sophgo/sg2000-milkv-duos.dts
new file mode 100644
index 000000000000..cf2cec3cc369
--- /dev/null
+++ b/arch/riscv/boot/dts/sophgo/sg2000-milkv-duos.dts
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/*
+ * Copyright (C) 2024 Michael Opdenacker <michael.opdenacker@bootlin.com>
+ */
+
+/dts-v1/;
+
+#include "cv1800b.dtsi"
+
+/ {
+ model = "Milk-V Duo S";
+ compatible = "milkv,duos", "sophgo,sg2000";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@80000000 {
+ device_type = "memory";
+ reg = <0x80000000 0x20000000>;
+ };
+};
+
+&osc {
+ clock-frequency = <25000000>;
+};
+
+&sdhci0 {
+ status = "okay";
+};
+
+&uart0 {
+ status = "okay";
+};
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] ASoC: dt-bindings: imx-audio-spdif: convert to YAML
From: Shengjiu Wang @ 2024-04-09 6:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Shengjiu Wang, lgirdwood, broonie, robh+dt,
krzysztof.kozlowski+dt, conor+dt, linux-sound, devicetree,
linux-kernel, shawnguo, s.hauer, kernel, festevam, imx,
linux-arm-kernel
In-Reply-To: <51e482b3-ffbf-46f9-972d-875d106c2474@linaro.org>
On Tue, Apr 9, 2024 at 2:30 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 09/04/2024 03:37, Shengjiu Wang wrote:
> > On Mon, Apr 8, 2024 at 11:06 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >>
> >> On 08/04/2024 10:01, Shengjiu Wang wrote:
> >>>>> +
> >>>>> +anyOf:
> >>>>> + - required:
> >>>>> + - spdif-in
> >>>>> + - required:
> >>>>> + - spdif-out
> >>>>> + - required:
> >>>>> + - spdif-out
> >>>>> + - spdif-in
> >>>>
> >>>> Do you need the last required block?
> >>>
> >>> Yes, one of them or both are required.
> >>
> >> And? It's already there: that's the meaning of any. It is not oneOf...
> >> Before answering please test your changes and ideas. I pointed issue
> >> here and you responded just to close my comment. That does not make me
> >> happy, just wastes my time.
> >
> > Maybe I didn't express clearly.
> >
> > we need at least one of them (spdif-in, spdif-out) in the node. which means
> > that we need to select "spdif-in", or "spdif-out", or "spdif-in and
> > spdif-out".
> >
> > So my understanding is that need to use "anyOf", if it is wrong, please let
> > me know.
>
> Third time: it is wrong.
>
> Can you test the code instead continuing this discussion
Every time I change I definitely do dtbs_check test.
But There is no error reported by dtbs_check.
But finally I understand that
anyOf:
- required:
- spdif-in
- required:
- spdif-out
- required:
- spdif-out
- spdif-in
is equal to:
anyOf:
- required:
- spdif-in
- required:
- spdif-out
best regards
Shengjiu Wang
^ permalink raw reply
* RE: [PATCH v4 2/2] phy: starfive: Add mipi dphy tx support
From: Shengyang Chen @ 2024-04-09 6:30 UTC (permalink / raw)
To: Vinod Koul
Cc: devicetree@vger.kernel.org, linux-phy@lists.infradead.org,
kishon@kernel.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
p.zabel@pengutronix.de, Minda Chen, Changhuang Liang,
rogerq@kernel.org, geert+renesas@glider.be, Keith Zhao,
linux-kernel@vger.kernel.org
In-Reply-To: <ZhAndLAItp8LhH_d@matsya>
Hi, Vinod
Thanks for review and comment.
> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: 2024年4月6日 0:32
> To: Shengyang Chen <shengyang.chen@starfivetech.com>
> Cc: devicetree@vger.kernel.org; linux-phy@lists.infradead.org;
> kishon@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> conor+dt@kernel.org; p.zabel@pengutronix.de; Minda Chen
> <minda.chen@starfivetech.com>; Changhuang Liang
> <changhuang.liang@starfivetech.com>; rogerq@kernel.org;
> geert+renesas@glider.be; Keith Zhao <keith.zhao@starfivetech.com>;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 2/2] phy: starfive: Add mipi dphy tx support
>
> On 01-03-24, 09:24, Shengyang Chen wrote:
> > Add mipi dphy tx support for the StarFive JH7110 SoC.
> > It is a module which is used to receive data from DSI driver and
> > transfer data to DSI interface like mipi screen.
> >
> > Signed-off-by: Shengyang Chen <shengyang.chen@starfivetech.com>
> > Reviewed-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> > ---
> > MAINTAINERS | 7 +
> > drivers/phy/starfive/Kconfig | 10 +
> > drivers/phy/starfive/Makefile | 1 +
> > drivers/phy/starfive/phy-jh7110-dphy-tx.c | 702
> > ++++++++++++++++++++++
> > 4 files changed, 720 insertions(+)
> > create mode 100644 drivers/phy/starfive/phy-jh7110-dphy-tx.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > ddbe6c82610f..13219c6504a9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20871,6 +20871,13 @@ S: Supported
> > F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-rx.yaml
> > F: drivers/phy/starfive/phy-jh7110-dphy-rx.c
> >
> > +STARFIVE JH7110 DPHY TX DRIVER
> > +M: Keith Zhao <keith.zhao@starfivetech.com>
> > +M: Shengyang Chen <shengyang.chen@starfivetech.com>
> > +S: Supported
> > +F: Documentation/devicetree/bindings/phy/starfive,jh7110-dphy-tx.yaml
> > +F: drivers/phy/starfive/phy-jh7110-dphy-tx.c
> > +
> > STARFIVE JH7110 MMC/SD/SDIO DRIVER
> > M: William Qiu <william.qiu@starfivetech.com>
> > S: Supported
> > diff --git a/drivers/phy/starfive/Kconfig
> > b/drivers/phy/starfive/Kconfig index 9508e2143011..d0cdd7cb4a13 100644
> > --- a/drivers/phy/starfive/Kconfig
> > +++ b/drivers/phy/starfive/Kconfig
> > @@ -15,6 +15,16 @@ config PHY_STARFIVE_JH7110_DPHY_RX
> > system. If M is selected, the module will be called
> > phy-jh7110-dphy-rx.ko.
> >
> > +config PHY_STARFIVE_JH7110_DPHY_TX
> > + tristate "StarFive JH7110 D-PHY TX Support"
> > + depends on HAS_IOMEM
> > + select GENERIC_PHY
> > + select GENERIC_PHY_MIPI_DPHY
> > + help
> > + Choose this option if you have a StarFive D-PHY TX in your
> > + system. If M is selected, the module will be called
> > + phy-jh7110-dphy-tx.ko.
> > +
> > config PHY_STARFIVE_JH7110_PCIE
> > tristate "Starfive JH7110 PCIE 2.0/USB 3.0 PHY support"
> > depends on HAS_IOMEM
> > diff --git a/drivers/phy/starfive/Makefile
> > b/drivers/phy/starfive/Makefile index b391018b7c47..eedc4a6fec15
> > 100644
> > --- a/drivers/phy/starfive/Makefile
> > +++ b/drivers/phy/starfive/Makefile
> > @@ -1,4 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_RX) += phy-jh7110-dphy-rx.o
> > +obj-$(CONFIG_PHY_STARFIVE_JH7110_DPHY_TX) += phy-jh7110-dphy-tx.o
> > obj-$(CONFIG_PHY_STARFIVE_JH7110_PCIE) += phy-jh7110-pcie.o
> > obj-$(CONFIG_PHY_STARFIVE_JH7110_USB) += phy-jh7110-usb.o
> > diff --git a/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> > b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> > new file mode 100644
> > index 000000000000..df30cd424ba7
> > --- /dev/null
> > +++ b/drivers/phy/starfive/phy-jh7110-dphy-tx.c
> > @@ -0,0 +1,702 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * DPHY TX driver for the StarFive JH7110 SoC
> > + *
> > + * Copyright (C) 2023 StarFive Technology Co., Ltd.
> > + * Author: Keith Zhao <keith.zhao@starfivetech.com>
> > + * Author: Shengyang Chen <shengyang.chen@starfivetech.com> */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/phy/phy-mipi-dphy.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/reset.h>
> > +
> > +#define STF_DPHY_APBIFSAIF_SYSCFG(x) (x)
> > +
> > +#define STF_DPHY_AON_POWER_READY_N_ACTIVE 0
> > +#define STF_DPHY_AON_POWER_READY_N_SHIFT 0x0U
> > +#define STF_DPHY_AON_POWER_READY_N_MASK BIT(0)
> > +#define STF_DPHY_CFG_L0_SWAP_SEL_SHIFT 0xcU
> > +#define STF_DPHY_CFG_L0_SWAP_SEL_MASK GENMASK(14,
> 12)
> > +#define STF_DPHY_CFG_L1_SWAP_SEL_SHIFT 0xfU
> > +#define STF_DPHY_CFG_L1_SWAP_SEL_MASK GENMASK(17,
> 15)
> > +#define STF_DPHY_CFG_L2_SWAP_SEL_SHIFT 0x12U
> > +#define STF_DPHY_CFG_L2_SWAP_SEL_MASK GENMASK(20,
> 18)
> > +#define STF_DPHY_CFG_L3_SWAP_SEL_SHIFT 0x15U
> > +#define STF_DPHY_CFG_L3_SWAP_SEL_MASK GENMASK(23,
> 21)
> > +#define STF_DPHY_CFG_L4_SWAP_SEL_SHIFT 0x18U
> > +#define STF_DPHY_CFG_L4_SWAP_SEL_MASK GENMASK(26,
> 24)
> > +#define STF_DPHY_RGS_CDTX_PLL_UNLOCK_SHIFT 0x12U
> > +#define STF_DPHY_RGS_CDTX_PLL_UNLOCK_MASK BIT(18)
> > +#define STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT 0x13U
> > +#define STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK GENMASK(23,
> 19)
> > +#define STF_DPHY_RG_CDTX_L0P_HSTX_RES_SHIFT 0x18U
> > +#define STF_DPHY_RG_CDTX_L0P_HSTX_RES_MASK GENMASK(28,
> 24)
> > +
> > +#define STF_DPHY_RG_CDTX_L1P_HSTX_RES_SHIFT 0x5U
> > +#define STF_DPHY_RG_CDTX_L1P_HSTX_RES_MASK GENMASK(9,
> 5)
> > +#define STF_DPHY_RG_CDTX_L2N_HSTX_RES_SHIFT 0xaU
> > +#define STF_DPHY_RG_CDTX_L2N_HSTX_RES_MASK GENMASK(14,
> 10)
> > +#define STF_DPHY_RG_CDTX_L2P_HSTX_RES_SHIFT 0xfU
> > +#define STF_DPHY_RG_CDTX_L2P_HSTX_RES_MASK GENMASK(19,
> 15)
> > +#define STF_DPHY_RG_CDTX_L3N_HSTX_RES_SHIFT 0x14U
> > +#define STF_DPHY_RG_CDTX_L3N_HSTX_RES_MASK GENMASK(24,
> 20)
> > +#define STF_DPHY_RG_CDTX_L3P_HSTX_RES_SHIFT 0x19U
> > +#define STF_DPHY_RG_CDTX_L3P_HSTX_RES_MASK GENMASK(29,
> 25)
>
> can you get rid of the shift defines. Please use FEILD_PREP and FIELD_GET which
> uses the bitmasks you have defined about (you can drop _MASK from above)
>
ok, will follow up this issue.
Will replace MASK with FIELD_PREP/FIELD_GET
> > +
> > +#define STF_DPHY_RG_CDTX_L4N_HSTX_RES_SHIFT 0x0U
> > +#define STF_DPHY_RG_CDTX_L4N_HSTX_RES_MASK GENMASK(4,
> 0)
> > +#define STF_DPHY_RG_CDTX_L4P_HSTX_RES_SHIFT 0x5U
> > +#define STF_DPHY_RG_CDTX_L4P_HSTX_RES_MASK GENMASK(9,
> 5)
> > +#define STF_DPHY_RG_CDTX_PLL_FBK_FRA_SHIFT 0x0U
> > +#define STF_DPHY_RG_CDTX_PLL_FBK_FRA_MASK GENMASK(23,
> 0)
> > +
> > +#define STF_DPHY_RG_CDTX_PLL_FBK_INT_SHIFT 0x0U
> > +#define STF_DPHY_RG_CDTX_PLL_FBK_INT_MASK GENMASK(8,
> 0)
> > +#define STF_DPHY_RG_CDTX_PLL_FM_EN_SHIFT 0x9U
> > +#define STF_DPHY_RG_CDTX_PLL_FM_EN_MASK BIT(9)
> > +#define STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_SHIFT 0xaU
> > +#define STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_MASK BIT(10)
> > +#define STF_DPHY_RG_CDTX_PLL_PRE_DIV_SHIFT 0xbU
> > +#define STF_DPHY_RG_CDTX_PLL_PRE_DIV_MASK GENMASK(12,
> 11)
> > +
> > +#define STF_DPHY_RG_CDTX_PLL_SSC_EN_SHIFT 0x12U
> > +#define STF_DPHY_RG_CDTX_PLL_SSC_EN_MASK 0x40000U
> > +
> > +#define STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_SHIFT 0x0U
> > +#define STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_MASK GENMASK(7,
> 0)
> > +#define STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_SHIFT 0x8U
> > +#define STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_MASK
> GENMASK(15, 8)
> > +#define STF_DPHY_RG_CLANE_HS_PRE_TIME_SHIFT 0x10U
> > +#define STF_DPHY_RG_CLANE_HS_PRE_TIME_MASK GENMASK(23,
> 16)
> > +#define STF_DPHY_RG_CLANE_HS_TRAIL_TIME_SHIFT 0x18U
> > +#define STF_DPHY_RG_CLANE_HS_TRAIL_TIME_MASK
> GENMASK(31, 24)
> > +
> > +#define STF_DPHY_RG_CLANE_HS_ZERO_TIME_SHIFT 0x0U
> > +#define STF_DPHY_RG_CLANE_HS_ZERO_TIME_MASK GENMASK(7,
> 0)
> > +#define STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT 0x8U
> > +#define STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK GENMASK(15,
> 8)
> > +#define STF_DPHY_RG_DLANE_HS_TRAIL_TIME_SHIFT 0x10U
> > +#define STF_DPHY_RG_DLANE_HS_TRAIL_TIME_MASK
> GENMASK(23, 16)
> > +#define STF_DPHY_RG_DLANE_HS_ZERO_TIME_SHIFT 0x18U
> > +#define STF_DPHY_RG_DLANE_HS_ZERO_TIME_MASK
> GENMASK(31, 24)
> > +
> > +#define STF_DPHY_RG_EXTD_CYCLE_SEL_SHIFT 0x0U
> > +#define STF_DPHY_RG_EXTD_CYCLE_SEL_MASK GENMASK(2, 0)
> > +#define STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_SHIFT 0x0U
> > +#define STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_MASK
> GENMASK(31, 0)
> > +
> > +#define STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_SHIFT 0x1U
> > +#define STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_MASK
> GENMASK(2, 1)
> > +#define STF_DPHY_SCFG_PPI_C_READY_SEL_SHIFT 0x3U
> > +#define STF_DPHY_SCFG_PPI_C_READY_SEL_MASK GENMASK(4,
> 3)
> > +
> > +#define STF_DPHY_REFCLK_IN_SEL_SHIFT 0x1aU
> > +#define STF_DPHY_REFCLK_IN_SEL_MASK GENMASK(28, 26)
> > +#define STF_DPHY_RESETB_SHIFT 0x1dU
> > +#define STF_DPHY_RESETB_MASK BIT(29)
> > +
> > +#define STF_DPHY_REFCLK_12M 1
> > +#define STF_DPHY_BITRATE_ALIGN 10000000
> > +
> > +#define STF_MAP_LANES_NUM 5
> > +
> > +#define STF_DPHY_LSHIFT_16(x) ((x) << 16)
> > +#define STF_DPHY_LSHIFT_8(x) ((x) << 8)
>
> these two should be dropped
>
Ok, will drop in next version
> > +
> > +struct m31_dphy_config {
> > + int ref_clk;
>
> why is this signed?
>
Ok, will drop in next version
> > + unsigned long bitrate;
> > + u32 pll_prev_div;
> > + u32 pll_fbk_int;
> > + u32 pll_fbk_fra;
> > + u32 extd_cycle_sel;
> > + u32 dlane_hs_pre_time;
> > + u32 dlane_hs_zero_time;
> > + u32 dlane_hs_trail_time;
> > + u32 clane_hs_pre_time;
> > + u32 clane_hs_zero_time;
> > + u32 clane_hs_trail_time;
> > + u32 clane_hs_clk_pre_time;
> > + u32 clane_hs_clk_post_time;
> > +};
> > +
> > +static const struct m31_dphy_config m31_dphy_configs[] = {
> > + {12000000, 160000000, 0x0, 0x6a,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x3, 0xa, 0x17, 0x11, 0x5, 0x2b, 0xd, 0x7, 0x3d},
>
> you can add this in a single line, that would be more readable...
>
ok, will follow up this issue.
> > + {12000000, 170000000, 0x0, 0x71,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x3, 0xb, 0x18, 0x11, 0x5, 0x2e, 0xd, 0x7, 0x3d},
> > + {12000000, 180000000, 0x0, 0x78,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x3, 0xb, 0x19, 0x12, 0x6, 0x30, 0xe, 0x7, 0x3e},
> > + {12000000, 190000000, 0x0, 0x7e,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x3, 0xc, 0x1a, 0x12, 0x6, 0x33, 0xe, 0x7, 0x3e},
> > + {12000000, 200000000, 0x0, 0x85,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x3, 0xc, 0x1b, 0x13, 0x7, 0x35, 0xf, 0x7, 0x3f},
> > + {12000000, 320000000, 0x0, 0x6a,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0x8, 0x14, 0xf, 0x5, 0x2b, 0xd, 0x3, 0x23},
> > + {12000000, 330000000, 0x0, 0x6e,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0x8, 0x15, 0xf, 0x5, 0x2d, 0xd, 0x3, 0x23},
> > + {12000000, 340000000, 0x0, 0x71,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0x9, 0x15, 0xf, 0x5, 0x2e, 0xd, 0x3, 0x23},
> > + {12000000, 350000000, 0x0, 0x74,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0x9, 0x15, 0x10, 0x6, 0x2f, 0xe, 0x3, 0x24},
> > + {12000000, 360000000, 0x0, 0x78,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0x9, 0x16, 0x10, 0x6, 0x30, 0xe, 0x3, 0x24},
> > + {12000000, 370000000, 0x0, 0x7b,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0x9, 0x17, 0x10, 0x6, 0x32, 0xe, 0x3, 0x24},
> > + {12000000, 380000000, 0x0, 0x7e,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xa, 0x17, 0x10, 0x6, 0x33, 0xe, 0x3, 0x24},
> > + {12000000, 390000000, 0x0, 0x82,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0, 0x2,
> > + 0xa, 0x17, 0x11, 0x6, 0x35, 0xf, 0x3, 0x25},
> > + {12000000, 400000000, 0x0, 0x85,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xa, 0x18, 0x11, 0x7, 0x35, 0xf, 0x3, 0x25},
> > + {12000000, 410000000, 0x0, 0x88,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xa, 0x19, 0x11, 0x7, 0x37, 0xf, 0x3, 0x25},
> > + {12000000, 420000000, 0x0, 0x8c,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xa, 0x19, 0x12, 0x7, 0x38, 0x10, 0x3, 0x26},
> > + {12000000, 430000000, 0x0, 0x8f,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xb, 0x19, 0x12, 0x7, 0x39, 0x10, 0x3, 0x26},
> > + {12000000, 440000000, 0x0, 0x92,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xb, 0x1a, 0x12, 0x7, 0x3b, 0x10, 0x3, 0x26},
> > + {12000000, 450000000, 0x0, 0x96,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xb, 0x1b, 0x12, 0x8, 0x3c, 0x10, 0x3, 0x26},
> > + {12000000, 460000000, 0x0, 0x99,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xb, 0x1b, 0x13, 0x8, 0x3d, 0x11, 0x3, 0x27},
> > + {12000000, 470000000, 0x0, 0x9c,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xc, 0x1b, 0x13, 0x8, 0x3e, 0x11, 0x3, 0x27},
> > + {12000000, 480000000, 0x0, 0xa0,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xc, 0x1c, 0x13, 0x8, 0x40, 0x11, 0x3, 0x27},
> > + {12000000, 490000000, 0x0, 0xa3,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xc, 0x1d, 0x14, 0x8, 0x42, 0x12, 0x3, 0x28},
> > + {12000000, 500000000, 0x0, 0xa6,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xc, 0x1d, 0x14, 0x9, 0x42, 0x12, 0x3, 0x28},
> > + {12000000, 510000000, 0x0, 0xaa,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xc, 0x1e, 0x14, 0x9, 0x44, 0x12, 0x3, 0x28},
> > + {12000000, 520000000, 0x0, 0xad,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xd, 0x1e, 0x15, 0x9, 0x45, 0x13, 0x3, 0x29},
> > + {12000000, 530000000, 0x0, 0xb0,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xd, 0x1e, 0x15, 0x9, 0x47, 0x13, 0x3, 0x29},
> > + {12000000, 540000000, 0x0, 0xb4,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xd, 0x1f, 0x15, 0x9, 0x48, 0x13, 0x3, 0x29},
> > + {12000000, 550000000, 0x0, 0xb7,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xd, 0x20, 0x16, 0x9, 0x4a, 0x14, 0x3, 0x2a},
> > + {12000000, 560000000, 0x0, 0xba,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xe, 0x20, 0x16, 0xa, 0x4a, 0x14, 0x3, 0x2a},
> > + {12000000, 570000000, 0x0, 0xbe,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xe, 0x20, 0x16, 0xa, 0x4c, 0x14, 0x3, 0x2a},
> > + {12000000, 580000000, 0x0, 0xc1,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xe, 0x21, 0x16, 0xa, 0x4d, 0x14, 0x3, 0x2a},
> > + {12000000, 590000000, 0x0, 0xc4,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xe, 0x22, 0x17, 0xa, 0x4f, 0x15, 0x3, 0x2b},
> > + {12000000, 600000000, 0x0, 0xc8,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x2, 0xe, 0x23, 0x17, 0xa, 0x50, 0x15, 0x3, 0x2b},
> > + {12000000, 610000000, 0x0, 0xcb,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x2, 0xf, 0x22, 0x17, 0xb, 0x50, 0x15, 0x3, 0x2b},
> > + {12000000, 620000000, 0x0, 0xce,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x2, 0xf, 0x23, 0x18, 0xb, 0x52, 0x16, 0x3, 0x2c},
> > + {12000000, 630000000, 0x0, 0x69,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x7, 0x12, 0xd, 0x5, 0x2a, 0xc, 0x1, 0x15},
> > + {12000000, 640000000, 0x0, 0x6a,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2b, 0xd, 0x1, 0x16},
> > + {12000000, 650000000, 0x0, 0x6c,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2c, 0xd, 0x1, 0x16},
> > + {12000000, 660000000, 0x0, 0x6e,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x7, 0x13, 0xe, 0x5, 0x2d, 0xd, 0x1, 0x16},
> > + {12000000, 670000000, 0x0, 0x6f,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x8, 0x13, 0xe, 0x5, 0x2d, 0xd, 0x1, 0x16},
> > + {12000000, 680000000, 0x0, 0x71,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x8, 0x13, 0xe, 0x5, 0x2e, 0xd, 0x1, 0x16},
> > + {12000000, 690000000, 0x0, 0x73,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x8, 0x14, 0xe, 0x6, 0x2e, 0xd, 0x1, 0x16},
> > + {12000000, 700000000, 0x0, 0x74,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x8, 0x14, 0xf, 0x6, 0x2f, 0xe, 0x1, 0x16},
> > + {12000000, 710000000, 0x0, 0x76,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x8, 0x14, 0xf, 0x6, 0x2f, 0xe, 0x1, 0x17},
> > + {12000000, 720000000, 0x0, 0x78,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x8, 0x15, 0xf, 0x6, 0x30, 0xe, 0x1, 0x17},
> > + {12000000, 730000000, 0x0, 0x79,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x8, 0x15, 0xf, 0x6, 0x31, 0xe, 0x1, 0x17},
> > + {12000000, 740000000, 0x0, 0x7b,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x8, 0x15, 0xf, 0x6, 0x32, 0xe, 0x1, 0x17},
> > + {12000000, 750000000, 0x0, 0x7d,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x8, 0x16, 0xf, 0x6, 0x32, 0xe, 0x1, 0x17},
> > + {12000000, 760000000, 0x0, 0x7e,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x9, 0x15, 0xf, 0x6, 0x33, 0xe, 0x1, 0x17},
> > + {12000000, 770000000, 0x0, 0x80,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x9, 0x15, 0x10, 0x6, 0x34, 0xf, 0x1, 0x18},
> > + {12000000, 780000000, 0x0, 0x82,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x9, 0x16, 0x10, 0x6, 0x35, 0xf, 0x1, 0x18,},
> > + {12000000, 790000000, 0x0, 0x83,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x9, 0x16, 0x10, 0x7, 0x34, 0xf, 0x1, 0x18},
> > + {12000000, 800000000, 0x0, 0x85,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x9, 0x17, 0x10, 0x7, 0x35, 0xf, 0x1, 0x18},
> > + {12000000, 810000000, 0x0, 0x87,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x9, 0x17, 0x10, 0x7, 0x36, 0xf, 0x1, 0x18},
> > + {12000000, 820000000, 0x0, 0x88,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0x9, 0x17, 0x10, 0x7, 0x37, 0xf, 0x1, 0x18},
> > + {12000000, 830000000, 0x0, 0x8a,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0x9, 0x18, 0x10, 0x7, 0x37, 0xf, 0x1, 0x18},
> > + {12000000, 840000000, 0x0, 0x8c,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0x9, 0x18, 0x11, 0x7, 0x38, 0x10, 0x1, 0x19},
> > + {12000000, 850000000, 0x0, 0x8d,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xa, 0x17, 0x11, 0x7, 0x39, 0x10, 0x1, 0x19},
> > + {12000000, 860000000, 0x0, 0x8f,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0xa, 0x18, 0x11, 0x7, 0x39, 0x10, 0x1, 0x19},
> > + {12000000, 870000000, 0x0, 0x91,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0xa, 0x18, 0x11, 0x7, 0x3a, 0x10, 0x1, 0x19},
> > + {12000000, 880000000, 0x0, 0x92,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xa, 0x18, 0x11, 0x7, 0x3b, 0x10, 0x1, 0x19},
> > + {12000000, 890000000, 0x0, 0x94,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0xa, 0x19, 0x11, 0x7, 0x3c, 0x10, 0x1, 0x19},
> > + {12000000, 900000000, 0x0, 0x96,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0xa, 0x19, 0x12, 0x8, 0x3c, 0x10, 0x1, 0x19},
> > + {12000000, 910000000, 0x0, 0x97,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xa, 0x19, 0x12, 0x8, 0x3c, 0x11, 0x1, 0x1a},
> > + {12000000, 920000000, 0x0, 0x99,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0xa, 0x1a, 0x12, 0x8, 0x3d, 0x11, 0x1, 0x1a},
> > + {12000000, 930000000, 0x0, 0x9b,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0xa, 0x1a, 0x12, 0x8, 0x3e, 0x11, 0x1, 0x1a},
> > + {12000000, 940000000, 0x0, 0x9c,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x3e, 0x11, 0x1, 0x1a},
> > + {12000000, 950000000, 0x0, 0x9e,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x3f, 0x11, 0x1, 0x1a},
> > + {12000000, 960000000, 0x0, 0xa0,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0xb, 0x1a, 0x12, 0x8, 0x40, 0x11, 0x1, 0x1a},
> > + {12000000, 970000000, 0x0, 0xa1,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x41, 0x12, 0x1, 0x1b},
> > + {12000000, 980000000, 0x0, 0xa3,
> > + STF_DPHY_LSHIFT_16(0x55) | STF_DPHY_LSHIFT_8(0x55) | 0x55,
> > + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x42, 0x12, 0x1, 0x1b},
> > + {12000000, 990000000, 0x0, 0xa5,
> > + STF_DPHY_LSHIFT_16(0x0) | STF_DPHY_LSHIFT_8(0x0) | 0x0,
> > + 0x1, 0xb, 0x1b, 0x13, 0x8, 0x42, 0x12, 0x1, 0x1b},
> > + {12000000, 1000000000, 0x0, 0xa6,
> > + STF_DPHY_LSHIFT_16(0xaa) | STF_DPHY_LSHIFT_8(0xaa) | 0xaa,
> > + 0x1, 0xb, 0x1c, 0x13, 0x9, 0x42, 0x12, 0x1, 0x1b}, };
> > +
> > +struct stf_dphy_info {
> > + /**
> > + * @maps:
> > + *
> > + * Physical lanes and logic lanes mapping table.
> > + *
> > + * The default order is:
> > + * [data lane 0, data lane 1, data lane 2, date lane 3, clk lane]
> > + */
> > + u8 maps[STF_MAP_LANES_NUM];
> > +};
> > +
> > +struct stf_dphy {
> > + struct device *dev;
> > + void __iomem *topsys;
> > + struct clk *txesc_clk;
> > + struct reset_control *sys_rst;
> > +
> > + struct phy_configure_opts_mipi_dphy config;
> > +
> > + struct phy *phy;
> > + const struct stf_dphy_info *info;
> > +};
> > +
> > +static inline u32 stf_dphy_get_reg(void __iomem *io_addr, u32 addr,
> > +u32 shift, u32 mask) {
> > + u32 tmp;
> > +
> > + tmp = readl(io_addr);
> > + tmp = (tmp & mask) >> shift;
> > + return tmp;
> > +}
> > +
> > +static inline void stf_dphy_set_reg(void __iomem *io_addr, u32 addr,
> > +u32 data, u32 shift, u32 mask) {
> > + u32 tmp;
> > +
> > + tmp = readl(io_addr + addr);
> > + tmp &= ~mask;
> > + tmp |= (data << shift) & mask;
> > + writel(tmp, (io_addr + addr));
> > +}
> > +
> > +static int is_pll_locked(struct stf_dphy *dphy) {
> > + int tmp = stf_dphy_get_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(8),
> > + STF_DPHY_RGS_CDTX_PLL_UNLOCK_SHIFT,
> > + STF_DPHY_RGS_CDTX_PLL_UNLOCK_MASK);
> > + return !tmp;
>
> why inverted value?
>
ok, will fix it
> > +}
> > +
> > +static void stf_dphy_hw_reset(struct stf_dphy *dphy, int assert) {
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(100),
> > + !assert, STF_DPHY_RESETB_SHIFT, STF_DPHY_RESETB_MASK);
> > +
> > + if (!assert) {
> > + while ((!is_pll_locked(dphy)))
> > + ;
>
> This is infinite, that is not acceptable. Please have a decent timeout for this...
>
ok, will follow up this issue.
We may use readl_poll_timeout_atomic() here in next version
> > + dev_err(dphy->dev, "MIPI dphy-tx # PLL Locked\n");
> > + }
> > +}
> > +
> > +static int stf_dphy_configure(struct phy *phy, union
> > +phy_configure_opts *opts) {
> > + struct stf_dphy *dphy = phy_get_drvdata(phy);
> > + const struct stf_dphy_info *info = dphy->info;
> > + u32 bitrate = opts->mipi_dphy.hs_clk_rate;
> > + const struct m31_dphy_config *p;
> > + unsigned long alignment;
> > + int i;
> > +
> > + bitrate = opts->mipi_dphy.hs_clk_rate;
> > +
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(8), 0x10,
> > + STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L0N_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L0N_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L2N_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L2N_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L3N_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L3N_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(16), 0x10,
> > + STF_DPHY_RG_CDTX_L4N_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L4N_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(8), 0x10,
> > + STF_DPHY_RG_CDTX_L0P_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L0P_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L1P_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L1P_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L2P_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L2P_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(12), 0x10,
> > + STF_DPHY_RG_CDTX_L3P_HSTX_RES_SHIFT,
> STF_DPHY_RG_CDTX_L3P_HSTX_RES_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(16), 0x10,
> > + STF_DPHY_RG_CDTX_L4P_HSTX_RES_SHIFT,
> > +STF_DPHY_RG_CDTX_L4P_HSTX_RES_MASK);
> > +
> > + alignment = STF_DPHY_BITRATE_ALIGN;
> > + if (bitrate % alignment)
> > + bitrate += alignment - (bitrate % alignment);
> > +
> > + p = m31_dphy_configs;
> > + for (i = 0; i < ARRAY_SIZE(m31_dphy_configs); i++, p++) {
> > + if (p->bitrate == bitrate) {
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(100),
> > + STF_DPHY_REFCLK_12M,
> STF_DPHY_REFCLK_IN_SEL_SHIFT,
> > + STF_DPHY_REFCLK_IN_SEL_MASK);
> > +
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0),
> > + STF_DPHY_AON_POWER_READY_N_ACTIVE,
> > + STF_DPHY_AON_POWER_READY_N_SHIFT,
> > + STF_DPHY_AON_POWER_READY_N_MASK);
> > +
> > + /*Lane setting*/
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[0],
> > + STF_DPHY_CFG_L0_SWAP_SEL_SHIFT,
> > + STF_DPHY_CFG_L0_SWAP_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[1],
> > + STF_DPHY_CFG_L1_SWAP_SEL_SHIFT,
> > + STF_DPHY_CFG_L1_SWAP_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[2],
> > + STF_DPHY_CFG_L2_SWAP_SEL_SHIFT,
> > + STF_DPHY_CFG_L2_SWAP_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[3],
> > + STF_DPHY_CFG_L3_SWAP_SEL_SHIFT,
> > + STF_DPHY_CFG_L3_SWAP_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(0), info->maps[4],
> > + STF_DPHY_CFG_L4_SWAP_SEL_SHIFT,
> > + STF_DPHY_CFG_L4_SWAP_SEL_MASK);
> > + /*PLL setting*/
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(28), 0x0,
> > + STF_DPHY_RG_CDTX_PLL_SSC_EN_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_SSC_EN_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(24), 0x1,
> > + STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_LDO_STB_X2_EN_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(24), 0x1,
> > + STF_DPHY_RG_CDTX_PLL_FM_EN_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_FM_EN_MASK);
> > +
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(24),
> > + p->pll_prev_div,
> STF_DPHY_RG_CDTX_PLL_PRE_DIV_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_PRE_DIV_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(24),
> > + p->pll_fbk_int,
> STF_DPHY_RG_CDTX_PLL_FBK_INT_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_FBK_INT_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(20),
> > + p->pll_fbk_fra,
> STF_DPHY_RG_CDTX_PLL_FBK_FRA_SHIFT,
> > + STF_DPHY_RG_CDTX_PLL_FBK_FRA_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(40),
> > + p->extd_cycle_sel,
> STF_DPHY_RG_EXTD_CYCLE_SEL_SHIFT,
> > + STF_DPHY_RG_EXTD_CYCLE_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(36),
> > + p->dlane_hs_pre_time,
> > + STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT,
> > + STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(36),
> > + p->dlane_hs_pre_time,
> > + STF_DPHY_RG_DLANE_HS_PRE_TIME_SHIFT,
> > + STF_DPHY_RG_DLANE_HS_PRE_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(36),
> > + p->dlane_hs_zero_time,
> > + STF_DPHY_RG_DLANE_HS_ZERO_TIME_SHIFT,
> > + STF_DPHY_RG_DLANE_HS_ZERO_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(36),
> > + p->dlane_hs_trail_time,
> > + STF_DPHY_RG_DLANE_HS_TRAIL_TIME_SHIFT,
> > + STF_DPHY_RG_DLANE_HS_TRAIL_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(32),
> > + p->clane_hs_pre_time,
> > + STF_DPHY_RG_CLANE_HS_PRE_TIME_SHIFT,
> > + STF_DPHY_RG_CLANE_HS_PRE_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(36),
> > + p->clane_hs_zero_time,
> > + STF_DPHY_RG_CLANE_HS_ZERO_TIME_SHIFT,
> > + STF_DPHY_RG_CLANE_HS_ZERO_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(32),
> > + p->clane_hs_trail_time,
> > + STF_DPHY_RG_CLANE_HS_TRAIL_TIME_SHIFT,
> > + STF_DPHY_RG_CLANE_HS_TRAIL_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(32),
> > + p->clane_hs_clk_pre_time,
> > + STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_SHIFT,
> > + STF_DPHY_RG_CLANE_HS_CLK_PRE_TIME_MASK);
> > + stf_dphy_set_reg(dphy->topsys,
> STF_DPHY_APBIFSAIF_SYSCFG(32),
> > + p->clane_hs_clk_post_time,
> > + STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_SHIFT,
> > + STF_DPHY_RG_CLANE_HS_CLK_POST_TIME_MASK);
> > +
> > + break;
> > + }
> > + }
> > +
> > + return 0;
>
> this is the only return. It will always return success, consider making this return
> void
>
In 'struct phy_ops', the call back '.configure' needs Integer return value.
Maybe we can only return 0 here. (T-T)
Function prototypes:
https://elixir.bootlin.com/linux/v6.9-rc3/source/include/linux/phy/phy.h#L103
struct phy_ops {
...
int (*configure)(struct phy *phy, union phy_configure_opts *opts);
...
};
> > +}
> > +
> > +static int stf_dphy_init(struct phy *phy) {
> > + struct stf_dphy *dphy = phy_get_drvdata(phy);
> > + int ret;
> > +
> > + stf_dphy_hw_reset(dphy, 0);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(48), 0,
> > + STF_DPHY_SCFG_PPI_C_READY_SEL_SHIFT,
> STF_DPHY_SCFG_PPI_C_READY_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(48), 0,
> > + STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_SHIFT,
> > + STF_DPHY_SCFG_DSI_TXREADY_ESC_SEL_MASK);
> > + stf_dphy_set_reg(dphy->topsys, STF_DPHY_APBIFSAIF_SYSCFG(44), 0x30,
> > + STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_SHIFT,
> > + STF_DPHY_SCFG_C_HS_PRE_ZERO_TIME_MASK);
> > +
> > + ret = clk_prepare_enable(dphy->txesc_clk);
> > + if (ret) {
> > + dev_err(dphy->dev, "Failed to prepare/enable txesc_clk\n");
> > + return ret;
> > + }
> > +
> > + ret = reset_control_deassert(dphy->sys_rst);
> > + if (ret) {
> > + dev_err(dphy->dev, "Failed to deassert sys_rst\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int stf_dphy_exit(struct phy *phy) {
> > + struct stf_dphy *dphy = phy_get_drvdata(phy);
> > + int ret;
> > +
> > + ret = reset_control_assert(dphy->sys_rst);
> > + if (ret) {
> > + dev_err(dphy->dev, "Failed to assert sys_rst\n");
> > + return ret;
> > + }
> > +
> > + clk_disable_unprepare(dphy->txesc_clk);
> > +
> > + stf_dphy_hw_reset(dphy, 1);
> > +
> > + return 0;
> > +}
> > +
> > +static int stf_dphy_power_on(struct phy *phy) {
> > + struct stf_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + return pm_runtime_resume_and_get(dphy->dev);
> > +}
> > +
> > +static int stf_dphy_validate(struct phy *phy, enum phy_mode mode, int
> submode,
> > + union phy_configure_opts *opts) {
> > + if (mode != PHY_MODE_MIPI_DPHY)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int stf_dphy_power_off(struct phy *phy) {
> > + struct stf_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + return pm_runtime_put_sync(dphy->dev); }
> > +
> > +static const struct phy_ops stf_dphy_ops = {
> > + .power_on = stf_dphy_power_on,
> > + .power_off = stf_dphy_power_off,
> > + .init = stf_dphy_init,
> > + .exit = stf_dphy_exit,
> > + .configure = stf_dphy_configure,
> > + .validate = stf_dphy_validate,
> > + .owner = THIS_MODULE,
> > +};
> > +
> > +static int stf_dphy_probe(struct platform_device *pdev) {
> > + struct phy_provider *phy_provider;
> > + struct stf_dphy *dphy;
> > +
> > + dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
> > + if (!dphy)
> > + return -ENOMEM;
> > +
> > + dphy->info = of_device_get_match_data(&pdev->dev);
> > +
> > + dphy->dev = &pdev->dev;
> > + dev_set_drvdata(&pdev->dev, dphy);
> > +
> > + dphy->topsys = devm_platform_ioremap_resource(pdev, 0);
> > + if (IS_ERR(dphy->topsys))
> > + return PTR_ERR(dphy->topsys);
> > +
> > + pm_runtime_enable(&pdev->dev);
> > +
> > + dphy->txesc_clk = devm_clk_get(&pdev->dev, "txesc");
> > + if (IS_ERR(dphy->txesc_clk))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->txesc_clk),
> > + "Failed to get txesc clock\n");
> > +
> > + dphy->sys_rst = devm_reset_control_get_exclusive(&pdev->dev, "sys");
> > + if (IS_ERR(dphy->sys_rst))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->sys_rst),
> > + "Failed to get sys reset\n");
> > +
> > + dphy->phy = devm_phy_create(&pdev->dev, NULL, &stf_dphy_ops);
> > + if (IS_ERR(dphy->phy))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dphy->phy),
> > + "Failed to create phy\n");
> > +
> > + phy_set_drvdata(dphy->phy, dphy);
> > +
> > + phy_provider = devm_of_phy_provider_register(&pdev->dev,
> of_phy_simple_xlate);
> > + if (IS_ERR(phy_provider))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(phy_provider),
> > + "Failed to register phy\n");
> > +
> > + return 0;
> > +}
> > +
> > +static const struct stf_dphy_info starfive_dphy_info = {
> > + .maps = {0, 1, 2, 3, 4},
> > +};
> > +
> > +static const struct of_device_id stf_dphy_dt_ids[] = {
> > + {
> > + .compatible = "starfive,jh7110-dphy-tx",
> > + .data = &starfive_dphy_info,
> > + },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, stf_dphy_dt_ids);
> > +
> > +static struct platform_driver stf_dphy_driver = {
> > + .driver = {
> > + .name = "starfive-dphy-tx",
> > + .of_match_table = stf_dphy_dt_ids,
> > + },
> > + .probe = stf_dphy_probe,
> > +};
> > +module_platform_driver(stf_dphy_driver);
> > +
> > +MODULE_AUTHOR("Keith Zhao <keith.zhao@starfivetech.com>");
> > +MODULE_AUTHOR("Shengyang Chen
> <shengyang.chen@starfivetech.com>");
> > +MODULE_DESCRIPTION("StarFive JH7110 DPHY TX driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.17.1
> >
> >
> > --
> > linux-phy mailing list
> > linux-phy@lists.infradead.org
> > https://lists.infradead.org/mailman/listinfo/linux-phy
>
> --
> ~Vinod
thanks.
Best Regards,
Shengyang
^ permalink raw reply
* [PATCH v3] arm64: dts: imx8ulp: add caam jr
From: Pankaj Gupta @ 2024-04-09 7:06 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer,
kernel, festevam, linux-imx, peng.fan, ping.bai, haibo.chen,
devicetree, linux-arm-kernel, linux-kernel
Cc: Pankaj Gupta, Varun Sethi
Add crypto node in device tree for:
- CAAM job-ring
Signed-off-by: Varun Sethi <v.sethi@nxp.com>
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 32 ++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index 8a6596d5a581..946f2b68d16f 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -252,6 +252,38 @@ pcc3: clock-controller@292d0000 {
#reset-cells = <1>;
};
+ crypto: crypto@292e0000 {
+ compatible = "fsl,sec-v4.0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x292e0000 0x10000>;
+ ranges = <0 0x292e0000 0x10000>;
+
+ sec_jr0: jr@1000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x1000 0x1000>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr1: jr@2000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x2000 0x1000>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr2: jr@3000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x3000 0x1000>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ sec_jr3: jr@4000 {
+ compatible = "fsl,sec-v4.0-job-ring";
+ reg = <0x4000 0x1000>;
+ interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
+
tpm5: tpm@29340000 {
compatible = "fsl,imx8ulp-tpm", "fsl,imx7ulp-tpm";
reg = <0x29340000 0x1000>;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 2/4] coresight: Add support for multiple output ports on the funnel
From: Suzuki K Poulose @ 2024-04-09 7:13 UTC (permalink / raw)
To: Tao Zhang, Mathieu Poirier, Alexander Shishkin, Konrad Dybcio,
Mike Leach, Rob Herring, Krzysztof Kozlowski
Cc: Jinlong Mao, Leo Yan, Greg Kroah-Hartman, coresight,
linux-arm-kernel, linux-kernel, devicetree, Tingwei Zhang,
Yuanfang Zhang, Trilok Soni, Song Chai, linux-arm-msm, andersson
In-Reply-To: <ffce4577-b0f9-4af3-a379-0385a02ddae8@quicinc.com>
Hi
On 29/03/2024 09:27, Tao Zhang wrote:
>
> On 3/22/2024 12:41 AM, Suzuki K Poulose wrote:
>> On 21/03/2024 08:32, Tao Zhang wrote:
>>> Funnel devices are now capable of supporting multiple-inputs and
>>> multiple-outputs configuration with in built hardware filtering
>>> for TPDM devices. Add software support to this function. Output
>>> port is selected according to the source in the trace path.
>>>
>>> The source of the input port on funnels will be marked in the
>>> device tree.
>>> e.g.
>>> tpdm@xxxxxxx {
>>> ... ... ... ...
>>> };
>>>
>>> funnel_XXX: funnel@xxxxxxx {
>>> ... ... ... ...
>>> out-ports {
>>> ... ... ... ...
>>> port@x {
>>> ... ... ... ...
>>> label = "xxxxxxx.tpdm"; <-- To label the source
>>> }; corresponding to the output
>>> ... ... ... ... connection "port@x". And this
>>> }; is a hardware static connections.
>>> ... ... ... ... Here needs to refer to hardware
>>> }; design.
>>>
>>> Then driver will parse the source label marked in the device tree, and
>>> save it to the coresight path. When the function needs to know the
>>> source label, it could obtain it from coresight path parameter. Finally,
>>> the output port knows which source it corresponds to, and it also knows
>>> which input port it corresponds to.
>>
>> Why do we need labels ? We have connection information for all devices
>> (both in and out), so, why do we need this label to find a device ?
>
> Because our funnel's design has multi-output ports, the data stream will
> not
>
> know which output port should pass in building the data trace path. This
> source
>
> label can make the data stream find the right output port to go.
>
>>
>> And also, I thought TPDM is a source device, why does a funnel output
>> port link to a source ?
>
> No, this label doesn't mean this funnel output port link to a source, it
> just let
>
> the output port know its data source.
>
>>
>> Are these funnels programmable ? Or, are they static ? If they are
>> static, do these need to be described in the DT ? If they are simply
>> acting as a "LINK" (or HWFIFO ?)
>
> These funnels are static, and we will add the "label" to the DT to
> describe the
>
> multi-output ports for these funnels.
I think there is still a bit of confusion. By "Dynamic" I mean,
the "dynamic funnel" (explicit port enablement via MMIO) vs "static
funnel" (no programming, always ON).
So, coming to your example, do we need to "explicitly" enable trace flow
for an "input" and/or an "output" port in your "funnel" ?
>
> "If they are simply acting as a "LINK" (or HWFIFO ?) " I'm not sure
> what's the meaning
i.e, Like TMC-ETF in HWFIFO mode. In this mode, the TMC-ETF is acting
like a cache for easing ATB data load, by providing h/w buffering.
(In your case, it may not be providing any buffering, it doesn't matter
either way, as it is not visible to the driver).
Suzuki
>
> of this. Could you describe it in detail?
>
>
> Best,
>
> Tao
>
>>
>> Suzuki
>>
>>>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-core.c | 81 ++++++++++++++++---
>>> .../hwtracing/coresight/coresight-platform.c | 5 ++
>>> include/linux/coresight.h | 2 +
>>> 3 files changed, 75 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-core.c
>>> b/drivers/hwtracing/coresight/coresight-core.c
>>> index 5dde597403b3..b1b5e6d9ec7a 100644
>>> --- a/drivers/hwtracing/coresight/coresight-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-core.c
>>> @@ -113,15 +113,63 @@ struct coresight_device
>>> *coresight_get_percpu_sink(int cpu)
>>> }
>>> EXPORT_SYMBOL_GPL(coresight_get_percpu_sink);
>>> +static struct coresight_device *coresight_get_source(struct
>>> list_head *path)
>>> +{
>>> + struct coresight_device *csdev;
>>> +
>>> + if (!path)
>>> + return NULL;
>>> +
>>> + csdev = list_first_entry(path, struct coresight_node, link)->csdev;
>>> + if (csdev->type != CORESIGHT_DEV_TYPE_SOURCE)
>>> + return NULL;
>>> +
>>> + return csdev;
>>> +}
>>> +
>>> +/**
>>> + * coresight_source_filter - checks whether the connection matches
>>> the source
>>> + * of path if connection is binded to specific source.
>>> + * @path: The list of devices
>>> + * @conn: The connection of one outport
>>> + *
>>> + * Return zero if the connection doesn't have a source binded or
>>> source of the
>>> + * path matches the source binds to connection.
>>> + */
>>> +static int coresight_source_filter(struct list_head *path,
>>> + struct coresight_connection *conn)
>>> +{
>>> + int ret = 0;
>>> + struct coresight_device *source = NULL;
>>> +
>>> + if (conn->source_label == NULL)
>>> + return ret;
>>> +
>>> + source = coresight_get_source(path);
>>> + if (source == NULL)
>>> + return ret;
>>> +
>>> + if (strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>> + conn->source_label))
>>> + ret = 0;
>>> + else
>>> + ret = -1;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> static struct coresight_connection *
>>> coresight_find_out_connection(struct coresight_device *src_dev,
>>> - struct coresight_device *dest_dev)
>>> + struct coresight_device *dest_dev,
>>> + struct list_head *path)
>>> {
>>> int i;
>>> struct coresight_connection *conn;
>>> for (i = 0; i < src_dev->pdata->nr_outconns; i++) {
>>> conn = src_dev->pdata->out_conns[i];
>>> + if (coresight_source_filter(path, conn))
>>> + continue;
>>> if (conn->dest_dev == dest_dev)
>>> return conn;
>>> }
>>> @@ -312,7 +360,8 @@ static void coresight_disable_sink(struct
>>> coresight_device *csdev)
>>> static int coresight_enable_link(struct coresight_device *csdev,
>>> struct coresight_device *parent,
>>> - struct coresight_device *child)
>>> + struct coresight_device *child,
>>> + struct list_head *path)
>>> {
>>> int ret = 0;
>>> int link_subtype;
>>> @@ -321,8 +370,8 @@ static int coresight_enable_link(struct
>>> coresight_device *csdev,
>>> if (!parent || !child)
>>> return -EINVAL;
>>> - inconn = coresight_find_out_connection(parent, csdev);
>>> - outconn = coresight_find_out_connection(csdev, child);
>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>> link_subtype = csdev->subtype.link_subtype;
>>> if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG &&
>>> IS_ERR(inconn))
>>> @@ -341,7 +390,8 @@ static int coresight_enable_link(struct
>>> coresight_device *csdev,
>>> static void coresight_disable_link(struct coresight_device *csdev,
>>> struct coresight_device *parent,
>>> - struct coresight_device *child)
>>> + struct coresight_device *child,
>>> + struct list_head *path)
>>> {
>>> int i;
>>> int link_subtype;
>>> @@ -350,8 +400,8 @@ static void coresight_disable_link(struct
>>> coresight_device *csdev,
>>> if (!parent || !child)
>>> return;
>>> - inconn = coresight_find_out_connection(parent, csdev);
>>> - outconn = coresight_find_out_connection(csdev, child);
>>> + inconn = coresight_find_out_connection(parent, csdev, path);
>>> + outconn = coresight_find_out_connection(csdev, child, path);
>>> link_subtype = csdev->subtype.link_subtype;
>>> if (link_ops(csdev)->disable) {
>>> @@ -507,7 +557,7 @@ static void coresight_disable_path_from(struct
>>> list_head *path,
>>> case CORESIGHT_DEV_TYPE_LINK:
>>> parent = list_prev_entry(nd, link)->csdev;
>>> child = list_next_entry(nd, link)->csdev;
>>> - coresight_disable_link(csdev, parent, child);
>>> + coresight_disable_link(csdev, parent, child, path);
>>> break;
>>> default:
>>> break;
>>> @@ -588,7 +638,7 @@ int coresight_enable_path(struct list_head *path,
>>> enum cs_mode mode,
>>> case CORESIGHT_DEV_TYPE_LINK:
>>> parent = list_prev_entry(nd, link)->csdev;
>>> child = list_next_entry(nd, link)->csdev;
>>> - ret = coresight_enable_link(csdev, parent, child);
>>> + ret = coresight_enable_link(csdev, parent, child, path);
>>> if (ret)
>>> goto err;
>>> break;
>>> @@ -802,7 +852,8 @@ static void coresight_drop_device(struct
>>> coresight_device *csdev)
>>> */
>>> static int _coresight_build_path(struct coresight_device *csdev,
>>> struct coresight_device *sink,
>>> - struct list_head *path)
>>> + struct list_head *path,
>>> + struct coresight_device *source)
>>> {
>>> int i, ret;
>>> bool found = false;
>>> @@ -814,7 +865,7 @@ static int _coresight_build_path(struct
>>> coresight_device *csdev,
>>> if (coresight_is_percpu_source(csdev) &&
>>> coresight_is_percpu_sink(sink) &&
>>> sink == per_cpu(csdev_sink,
>>> source_ops(csdev)->cpu_id(csdev))) {
>>> - if (_coresight_build_path(sink, sink, path) == 0) {
>>> + if (_coresight_build_path(sink, sink, path, source) == 0) {
>>> found = true;
>>> goto out;
>>> }
>>> @@ -825,8 +876,12 @@ static int _coresight_build_path(struct
>>> coresight_device *csdev,
>>> struct coresight_device *child_dev;
>>> child_dev = csdev->pdata->out_conns[i]->dest_dev;
>>> + if (csdev->pdata->out_conns[i]->source_label &&
>>> + !strstr(kobject_get_path(&source->dev.kobj, GFP_KERNEL),
>>> + csdev->pdata->out_conns[i]->source_label))
>>> + continue;
>>> if (child_dev &&
>>> - _coresight_build_path(child_dev, sink, path) == 0) {
>>> + _coresight_build_path(child_dev, sink, path, source) ==
>>> 0) {
>>> found = true;
>>> break;
>>> }
>>> @@ -871,7 +926,7 @@ struct list_head *coresight_build_path(struct
>>> coresight_device *source,
>>> INIT_LIST_HEAD(path);
>>> - rc = _coresight_build_path(source, sink, path);
>>> + rc = _coresight_build_path(source, sink, path, source);
>>> if (rc) {
>>> kfree(path);
>>> return ERR_PTR(rc);
>>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>>> b/drivers/hwtracing/coresight/coresight-platform.c
>>> index 9d550f5697fa..f553fb20966d 100644
>>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>>> @@ -205,6 +205,7 @@ static int of_coresight_parse_endpoint(struct
>>> device *dev,
>>> struct fwnode_handle *rdev_fwnode;
>>> struct coresight_connection conn = {};
>>> struct coresight_connection *new_conn;
>>> + const char *label;
>>> do {
>>> /* Parse the local port details */
>>> @@ -243,6 +244,10 @@ static int of_coresight_parse_endpoint(struct
>>> device *dev,
>>> conn.dest_fwnode = fwnode_handle_get(rdev_fwnode);
>>> conn.dest_port = rendpoint.port;
>>> + conn.source_label = NULL;
>>> + if (!of_property_read_string(ep, "label", &label))
>>> + conn.source_label = label;
>>> +
>>> new_conn = coresight_add_out_conn(dev, pdata, &conn);
>>> if (IS_ERR_VALUE(new_conn)) {
>>> fwnode_handle_put(conn.dest_fwnode);
>>> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
>>> index e8b6e388218c..a9c06ef9bbb2 100644
>>> --- a/include/linux/coresight.h
>>> +++ b/include/linux/coresight.h
>>> @@ -167,6 +167,7 @@ struct coresight_desc {
>>> * struct coresight_connection - representation of a single connection
>>> * @src_port: a connection's output port number.
>>> * @dest_port: destination's input port number @src_port is
>>> connected to.
>>> + * @source_label: source component's label.
>>> * @dest_fwnode: destination component's fwnode handle.
>>> * @dest_dev: a @coresight_device representation of the component
>>> connected to @src_port. NULL until the device is created
>>> @@ -195,6 +196,7 @@ struct coresight_desc {
>>> struct coresight_connection {
>>> int src_port;
>>> int dest_port;
>>> + const char *source_label;
>>> struct fwnode_handle *dest_fwnode;
>>> struct coresight_device *dest_dev;
>>> struct coresight_sysfs_link *link;
>>
^ permalink raw reply
* Re: [PATCH] riscv: dts: sophgo: add initial Milk-V Duo S board device tree
From: Inochi Amaoto @ 2024-04-09 7:17 UTC (permalink / raw)
To: michael.opdenacker, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen Wang, Inochi Amaoto, Paul Walmsley,
Palmer Dabbelt, Albert Ou
Cc: devicetree, linux-riscv, linux-kernel
In-Reply-To: <20240409064504.4010353-1-michael.opdenacker@bootlin.com>
On Tue, Apr 09, 2024 at 08:45:04AM +0200, michael.opdenacker@bootlin.com wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>
> This adds initial support for the Milk-V Duo S board
> (https://milkv.io/duo-s), enabling the serial port and
> read-only SD card support, allowing to boot Linux to the
> command line.
>
Hi Michael,
I think your patch losts the board binding and have wrong compatiable.
Also, the SD can have rw support with proper property. See link [1]
(need 'disable-wp').
I suggest you resubmitting this patch after applying clk patchs.
This could reduce some unnecessary change. For now, you can just
use the dts for huashan-pi (with changed memory size).
[1]: https://lore.kernel.org/linux-riscv/IA1PR20MB4953B158F6F575840F3D4267BB7D2@IA1PR20MB4953.namprd20.prod.outlook.com/
^ permalink raw reply
* Re: [PATCH v3] arm64: dts: imx8ulp: add caam jr
From: Krzysztof Kozlowski @ 2024-04-09 7:26 UTC (permalink / raw)
To: Pankaj Gupta, robh+dt, krzysztof.kozlowski+dt, conor+dt, shawnguo,
s.hauer, kernel, festevam, linux-imx, peng.fan, ping.bai,
haibo.chen, devicetree, linux-arm-kernel, linux-kernel
Cc: Varun Sethi
In-Reply-To: <20240409070616.3868152-1-pankaj.gupta@nxp.com>
On 09/04/2024 09:06, Pankaj Gupta wrote:
> Add crypto node in device tree for:
> - CAAM job-ring
>
> Signed-off-by: Varun Sethi <v.sethi@nxp.com>
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
v3? Where is the changelog? What was happening here?
> ---
> arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 32 ++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> index 8a6596d5a581..946f2b68d16f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
> @@ -252,6 +252,38 @@ pcc3: clock-controller@292d0000 {
> #reset-cells = <1>;
> };
>
> + crypto: crypto@292e0000 {
> + compatible = "fsl,sec-v4.0";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + reg = <0x292e0000 0x10000>;
> + ranges = <0 0x292e0000 0x10000>;
Keep the order as requested in DTS coding style.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v14 2/4] dt-bindings: remoteproc: add Tightly Coupled Memory (TCM) bindings
From: Krzysztof Kozlowski @ 2024-04-09 7:29 UTC (permalink / raw)
To: Tanmay Shah, andersson, mathieu.poirier, robh,
krzysztof.kozlowski+dt, conor+dt, michal.simek, ben.levinsky
Cc: linux-remoteproc, devicetree, linux-arm-kernel, linux-kernel,
Radhey Shyam Pandey
In-Reply-To: <20240408205313.3552165-3-tanmay.shah@amd.com>
On 08/04/2024 22:53, Tanmay Shah wrote:
> From: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
>
> Introduce bindings for TCM memory address space on AMD-xilinx Zynq
> UltraScale+ platform. It will help in defining TCM in device-tree
> and make it's access platform agnostic and data-driven.
>
> Tightly-coupled memories(TCMs) are low-latency memory that provides
> predictable instruction execution and predictable data load/store
> timing. Each Cortex-R5F processor contains two 64-bit wide 64 KB memory
> banks on the ATCM and BTCM ports, for a total of 128 KB of memory.
>
> The TCM resources(reg, reg-names and power-domain) are documented for
> each TCM in the R5 node. The reg and reg-names are made as required
> properties as we don't want to hardcode TCM addresses for future
> platforms and for zu+ legacy implementation will ensure that the
> old dts w/o reg/reg-names works and stable ABI is maintained.
>
> It also extends the examples for TCM split and lockstep modes.
>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 1/9] dt-bindings: rtc: orion-rtc: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:31 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-1-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> This RTC requires a compatible, a reg and a single interrupt,
> which makes it suitable for a direct conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Documentation/devicetree/bindings/rtc/orion-rtc.txt | 18 ------------------
> Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
> 2 files changed, 2 insertions(+), 18 deletions(-)
I assume you checked if any entry in MAINTAINERS needs updating.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/9] dt-bindings: rtc: google,goldfish-rtc: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:32 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-2-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> This RTC requires a compatible, a reg and a single interrupt,
> which makes it suitable for a direct conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: rtc: lpc32xx-rtc: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:34 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-3-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> This RTC requires a compatible, a reg and a single interrupt,
> which makes it suitable for a direct conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Documentation/devicetree/bindings/rtc/lpc32xx-rtc.txt | 15 ---------------
> Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
> 2 files changed, 2 insertions(+), 15 deletions(-)
This one no... and if you tested DTS you would see errors, although you
need to test specific lpc config, not multi_v7.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Anyway, you *must* check all DTS before moving anything to trivial.
Does it mean all other bindings were not checked against DTS at all?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/9] dt-bindings: rtc: maxim,ds1742: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:35 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-4-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> This RTC only requires the compatible a reg and properties,
> which makes it suitable for a direct conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 5/9] dt-bindings: rtc: rtc-aspeed: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:36 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-5-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> The RTCs documented in this binding require a compatible, a reg
> and a single interrupt, which make them suitable for a direct
> conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 6/9] dt-bindings: rtc: pxa-rtc: convert to dtschema
From: Krzysztof Kozlowski @ 2024-04-09 7:37 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-6-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> The missing 'reg' and 'interrupts' properties have been added, taking
> the 2 supported interrupts into account to fix the example.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 7/9] dt-bindings: rtc: spear-rtc: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:37 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-7-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> The RTC documented in this binding requires a compatible, a reg
> and a single interrupt, which makes it suitable for a direct
> conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> Documentation/devicetree/bindings/rtc/spear-rtc.txt | 15 ---------------
> Documentation/devicetree/bindings/rtc/trivial-rtc.yaml | 2 ++
> 2 files changed, 2 insertions(+), 15 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 8/9] dt-bindings: rtc: stmp3xxx-rtc: convert to dtschema
From: Krzysztof Kozlowski @ 2024-04-09 7:40 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-8-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> Convert existing binding to dtschema to support validation.
>
> The 'fsl,imx28-rtc' compatible is currently not supported, and it is
> only referenced in this binding and in nxp/mxs/imx28.dtsi. Therefore,
> that compatible has been dropped, which triggers a warning when testing
> the DT against the new binding.
Instead document missing compatibles and mention this in commit msg.
>
> There is another reference to fsl,stmp3xxx-rtc in nxp/mxs/imx23.dtsi,
> where another unsupported compatible 'fsl,imx23-rtc' is used, and the
> same problem would arise when testing the file against the new binding.
Please write concise messages... you have to paragraphs about the same?
What is the difference here?
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
> .../devicetree/bindings/rtc/fsl,stmp3xxx-rtc.yaml | 45 ++++++++++++++++++++++
> .../devicetree/bindings/rtc/stmp3xxx-rtc.txt | 21 ----------
> 2 files changed, 45 insertions(+), 21 deletions(-)
>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 9/9] dt-bindings: rtc: via,vt8500-rtc: move to trivial-rtc
From: Krzysztof Kozlowski @ 2024-04-09 7:40 UTC (permalink / raw)
To: Javier Carrasco, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jiaxun Yang,
Vladimir Zapolskiy, Joel Stanley, Andrew Jeffery, Maxime Coquelin,
Alexandre Torgue
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed, linux-stm32
In-Reply-To: <20240408-rtc_dtschema-v1-9-c447542fc362@gmail.com>
On 08/04/2024 17:53, Javier Carrasco wrote:
> The RTC documented in this binding requires a compatible, a reg
> and a single interrupt, which makes it suitable for a direct
> conversion into trivial-rtc.
>
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v7 1/5] dt-bindings: interconnect: Add Qualcomm IPQ9574 support
From: Varadarajan Narayanan @ 2024-04-09 7:41 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: andersson, konrad.dybcio, mturquette, sboyd, robh, krzk+dt,
conor+dt, djakov, dmitry.baryshkov, quic_anusha, linux-arm-msm,
linux-clk, devicetree, linux-kernel, linux-pm
In-Reply-To: <Zg5q6mnWtK6hmPBT@hu-varada-blr.qualcomm.com>
On Thu, Apr 04, 2024 at 02:25:06PM +0530, Varadarajan Narayanan wrote:
> On Wed, Apr 03, 2024 at 04:59:40PM +0200, Krzysztof Kozlowski wrote:
> > On 03/04/2024 12:42, Varadarajan Narayanan wrote:
> > > Add interconnect-cells to clock provider so that it can be
> > > used as icc provider.
> > >
> > > Add master/slave ids for Qualcomm IPQ9574 Network-On-Chip
> > > interfaces. This will be used by the gcc-ipq9574 driver
> > > that will for providing interconnect services using the
> > > icc-clk framework.
> > >
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > > v7:
> > > Fix macro names to be consistent with other bindings
> > > v6:
> > > Removed Reviewed-by: Krzysztof Kozlowski
> > > Redefine the bindings such that driver and DT can share them
> > >
> > > v3:
> > > Squash Documentation/ and include/ changes into same patch
> > >
> > > qcom,ipq9574.h
> > > Move 'first id' to clock driver
> > >
> > > ---
> > > .../bindings/clock/qcom,ipq9574-gcc.yaml | 3 +
> > > .../dt-bindings/interconnect/qcom,ipq9574.h | 87 +++++++++++++++++++
> > > 2 files changed, 90 insertions(+)
> > > create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > > index 944a0ea79cd6..824781cbdf34 100644
> > > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > > @@ -33,6 +33,9 @@ properties:
> > > - description: PCIE30 PHY3 pipe clock source
> > > - description: USB3 PHY pipe clock source
> > >
> > > + '#interconnect-cells':
> > > + const: 1
> > > +
> > > required:
> > > - compatible
> > > - clocks
> > > diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > > new file mode 100644
> > > index 000000000000..0b076b0cf880
> > > --- /dev/null
> > > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > > @@ -0,0 +1,87 @@
> > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > > +#ifndef INTERCONNECT_QCOM_IPQ9574_H
> > > +#define INTERCONNECT_QCOM_IPQ9574_H
> > > +
> > > +#define ICC_ANOC_PCIE0 0
> > > +#define ICC_SNOC_PCIE0 1
> > > +#define ICC_ANOC_PCIE1 2
> > > +#define ICC_SNOC_PCIE1 3
> > > +#define ICC_ANOC_PCIE2 4
> > > +#define ICC_SNOC_PCIE2 5
> > > +#define ICC_ANOC_PCIE3 6
> > > +#define ICC_SNOC_PCIE3 7
> > > +#define ICC_SNOC_USB 8
> > > +#define ICC_ANOC_USB_AXI 9
> > > +#define ICC_NSSNOC_NSSCC 10
> > > +#define ICC_NSSNOC_SNOC_0 11
> > > +#define ICC_NSSNOC_SNOC_1 12
> > > +#define ICC_NSSNOC_PCNOC_1 13
> > > +#define ICC_NSSNOC_QOSGEN_REF 14
> > > +#define ICC_NSSNOC_TIMEOUT_REF 15
> > > +#define ICC_NSSNOC_XO_DCD 16
> > > +#define ICC_NSSNOC_ATB 17
> > > +#define ICC_MEM_NOC_NSSNOC 18
> > > +#define ICC_NSSNOC_MEMNOC 19
> > > +#define ICC_NSSNOC_MEM_NOC_1 20
> > > +
> > > +#define ICC_NSSNOC_PPE 0
> > > +#define ICC_NSSNOC_PPE_CFG 1
> > > +#define ICC_NSSNOC_NSS_CSR 2
> > > +#define ICC_NSSNOC_IMEM_QSB 3
> > > +#define ICC_NSSNOC_IMEM_AHB 4
> > > +
> > > +#define MASTER_ANOC_PCIE0 (ICC_ANOC_PCIE0 * 2)
> > > +#define SLAVE_ANOC_PCIE0 ((ICC_ANOC_PCIE0 * 2) + 1)
> >
> > Which existing Qualcomm platform has such code?
>
> Existing Qualcomm platforms don't use icc-clk. They use icc-rpm
> or icc-rpmh. clk-cbf-msm8996.c is the only driver that uses icc-clk.
>
> The icc_clk_register automatically creates master & slave nodes
> for each clk entry provided as input with the node-ids 'n' and
> 'n+1'. Since clk-cbf-msm8996.c has only one entry, it could just
> define MASTER_CBF_M4M and SLAVE_CBF_M4M with 0 and 1 and avoid these
> calculations.
>
> However, ipq9574 gives an array of clock entries as input to
> icc_clk_register. To tie the order/sequence of these clock
> entries correctly with the node-ids, this calculation is needed.
>
> > This is the third time I am asking for consistent headers. Open
> > existing, recently added headers and look how it is done there. Why?
> > Because I am against such calculations and see no reason for them.
>
> Apologies. Regret that I have to trouble you.
>
> In this ipq9574 case, have to reconcile between the following
> feedbacks.
>
> 1. https://lore.kernel.org/linux-arm-msm/fe40b307-26d0-4b2a-869b-5d093415b9d1@linaro.org/
> We could probably use indexed identifiers here to avoid confusion:
> [ICC_BINDING_NAME] = CLK_BINDING_NAME
>
> 2. https://lore.kernel.org/linux-arm-msm/95f4e99a60cc97770fc3cee850b62faf.sboyd@kernel.org/
> Are these supposed to be in a dt-binding header?
>
> 3. https://lore.kernel.org/linux-arm-msm/031d0a35-b192-4161-beef-97b89d5d1da6@linaro.org/
> Do you use them as well in the DTS?
>
> Having the defines (with the calculations) seemed to to comply
> with the above three feedbacks.
>
> Please let me know if this can be handled in a different way that
> would be consistent with other Qualcomm platforms.
Krzysztof,
Is this ok? Can I post a new version addressing other review comments?
Thanks
Varada
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: mailbox: arm,mhuv3: Add bindings
From: Krzysztof Kozlowski @ 2024-04-09 7:44 UTC (permalink / raw)
To: Cristian Marussi, linux-kernel, linux-arm-kernel, devicetree
Cc: sudeep.holla, jassisinghbrar, robh+dt, krzysztof.kozlowski+dt,
conor+dt
In-Reply-To: <20240404062347.3219795-2-cristian.marussi@arm.com>
On 04/04/2024 08:23, Cristian Marussi wrote:
> Add bindings for the ARM MHUv3 Mailbox controller.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
...
> + mailbox@2aaa0000 {
> + compatible = "arm,mhuv3";
> + #mbox-cells = <3>;
> + reg = <0 0x2aaa0000 0 0x10000>;
> + clocks = <&clock 0>;
> + interrupt-names = "combined", "pbx-dbch-xfer-ack-1",
> + "ffch-high-tide-0";
> + interrupts = <0 36 4>, <0 37 4>;
> + };
> +
> + mailbox@2ab00000 {
> + compatible = "arm,mhuv3";
> + #mbox-cells = <3>;
> + reg = <0 0x2aab0000 0 0x10000>;
> + clocks = <&clock 0>;
> + interrupt-names = "combined", "mbx-dbch-xfer-1", "ffch-low-tide-0";
> + interrupts = <0 35 4>, <0 38 4>, <0 39 4>;
Use defines for GIC and level flags.
With this fixed:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] riscv: dts: sophgo: add initial Milk-V Duo S board device tree
From: Michael Opdenacker @ 2024-04-09 7:52 UTC (permalink / raw)
To: Inochi Amaoto, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Chen Wang, Paul Walmsley, Palmer Dabbelt, Albert Ou
Cc: michael.opdenacker, devicetree, linux-riscv, linux-kernel
In-Reply-To: <IA1PR20MB49532A8D4294ADCE0C33A654BB072@IA1PR20MB4953.namprd20.prod.outlook.com>
Hi Inochi
Thanks for the review and tips!
On 4/9/24 at 09:17, Inochi Amaoto wrote:
> On Tue, Apr 09, 2024 at 08:45:04AM +0200, michael.opdenacker@bootlin.com wrote:
>> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>>
>> This adds initial support for the Milk-V Duo S board
>> (https://milkv.io/duo-s), enabling the serial port and
>> read-only SD card support, allowing to boot Linux to the
>> command line.
>>
> Hi Michael,
>
> I think your patch losts the board binding and have wrong compatiable.
> Also, the SD can have rw support with proper property. See link [1]
> (need 'disable-wp').
Right, ""sophgo,sg2000" doesn't exist yet, so it will be indeed cleaner
to use "sophgo,cv1812h" instead.
>
> I suggest you resubmitting this patch after applying clk patchs.
> This could reduce some unnecessary change. For now, you can just
> use the dts for huashan-pi (with changed memory size).
>
> [1]: https://lore.kernel.org/linux-riscv/IA1PR20MB4953B158F6F575840F3D4267BB7D2@IA1PR20MB4953.namprd20.prod.outlook.com/
Good to know. That was next on my list.
I'll submit a V2 in the next days.
Thanks again
Cheers
Michael.
--
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Ceclan, Dumitru @ 2024-04-09 8:08 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20240406155328.447b594f@jic23-huawei>
On 06/04/2024 17:53, Jonathan Cameron wrote:
> On Wed, 3 Apr 2024 10:40:39 -0500
> David Lechner <dlechner@baylibre.com> wrote:
>
>> On Wed, Apr 3, 2024 at 2:43 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
>>>
>>> On 01/04/2024 22:37, David Lechner wrote:
>>>> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
>>>> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>>>>
>>>>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>>
>>> ...
>>>
>>>>> properties:
>>>>> reg:
>>>>> + description:
>>>>> + Reg values 16-19 are only permitted for ad4111/ad4112 current channels.
>>>>> minimum: 0
>>>>> - maximum: 15
>>>>> + maximum: 19
>>>>
>>>> This looks wrong. Isn't reg describing the number of logical channels
>>>> (# of channel config registers)?
>>>>
>>>> After reviewing the driver, I see that > 16 is used as a way of
>>>> flagging current inputs, but still seems like the wrong way to do it.
>>>> See suggestion below.
>>>>
>>>
>>> This was a suggestion from Jonathan, maybe I implemented it wrong.
>
> Maybe Jonathan was wrong! I was younger then than now :)
>
> However, reg values for child nodes are unique so can't just use a flag these
> need to be different values.
>
I do not see where the restriction appears when using just the flag, when defining
the channels you would still define unique reg values.
>>> Other alternative that came to my mind: attribute "adi,current-channel".
>>
>> Having a boolean flag like this would make more sense to me if we
>> don't agree that the suggestion below is simpler.
>>
>>>>>
>>>>> diff-channels:
>>>>> + description:
>>>>> + For using current channels specify only the positive channel.
>>>>> + (IIN2+, IIN2−) -> diff-channels = <2 0>
>>>>
>>>> I find this a bit confusing since 2 is already VIN2 and 0 is already
>>>> VIN0. I think it would make more sense to assign unique channel
>>>> numbers individually to the negative and positive current inputs.
>>>> Also, I think it makes sense to use the same numbers that the
>>>> registers in the datasheet use (8 - 11 for negative and 12 to 15 for
>>>> positive).
>>>>
>>>> So: (IIN2+, IIN2−) -> diff-channels = <13 10>
>>>>
>>>>
>>> It would mean for the user to look in the datasheet at the possible
>>> channel INPUT configurations values, decode the bit field into two
>>> integer values and place it here (0110101010) -> 13 10. This is
>>> cumbersome for just choosing current input 2.
>>
>> It could be documented in the devicetree bindings, just as it is done
>> in adi,ad4130.yaml so that users of the bindings don't have to
>> decipher the datasheet.
>
> The <13 10> option makes sense to me and avoids suggesting a common negative
> input.
>
> The 'fun' bit here is that diff-channels doesn't actually tell us anything.
> So we could just not provide it and rely on documentation of reg = 16-19 meaning
> the current channels?
>
So a channel without diff-channels defined and reg=16 means IN0+ IN0-?
>>
>>>
>>>>> +
>>>>> + Family AD411x supports a dedicated VCOM voltage input.
>>>>> + To select it set the second channel to 16.
>>>>> + (VIN2, VCOM) -> diff-channels = <2 16>
>>>>
>>>> The 411x datasheets call this pin VINCOM so calling it VCOM here is a
>>>> bit confusing.
>>>>
>>>
>>> Sure, I'll rename to VINCOM.
>>>
>>>> Also, do we need to add a vincom-supply to get this voltage? Or is it
>>>> safe to assume it is always connected to AVSS? The datasheet seems to
>>>> indicate that the latter is the case. But then it also has this
>>>> special case (at least for AD4116, didn't check all datasheets)
>>>> "VIN10, VINCOM (single-ended or differential pair)". If it can be used
>>>> as part of a fully differential input, we probably need some extra
>>>> flag to indicate that case.
>>>>
>>>
>>> I cannot see any configuration options for these use cases. All inputs
>>> are routed to the same mux and routed to the differential positive and
>>> negative ADC inputs.
>>>
>>> "VIN10, VINCOM (single-ended or differential pair)" the only difference
>>> between these two use cases is if you connected VINCOM to AVSS (with
>>> unipolar coding) or not with bipolar encoding. The channel is still
>>> measuring the difference between the two selected inputs and comparing
>>> to the selected reference.
>>>
>>>> Similarly, do we need special handling for ADCIN15 on AD4116? It has a
>>>> "(pseudo differential or differential pair)" notation that other
>>>> inputs don't. In other words, it is more like VINCOM than it is to the
>>>> other ADCINxx pins. So we probably need an adcin15-supply for this pin
>>>> to properly get the right channel configuration. I.e. the logic in the
>>>> IIO driver would be if adcin15-supply is present, any channels that
>>>> use this input are pseudo-differential, otherwise any channels that
>>>> use it are fully differential.
>>>>
>>>
>>> I cannot seem to understand what would a adcin15-supply be needed for.
>>> This input, the same as all others, enters the mux and is routed to
>>> either positive or negative input of the ADC.
>>>
>>> The voltage on the ADCIN15 pin is not important to the user, just the
>>> difference in voltage between that pin and the other one selected.
>>>
>>
>> These suggestions come from some recent discussion about
>> pseudo-differential vs. fully differential inputs (e.g. search the IIO
>> mailing list for AD7380).
>>
>> So what I suggested here might be more technically correct according
>> to what I got out of that discussion. But for this specific case, I
>> agree it is good enough to just treat all inputs as always
>> fully-differential to keep things from getting too unwieldy.
>
> Hmm. That whole approach to pseudo differential does get messy if
> we have the common line routed through the main MUX rather than an opt
> in only on the negative side.
>
> If I read this right, its almost a trick to support a pseudo differential
> wiring with simple registers (I guess reflecting MUX limitations).
>
> So what could we do?
>
> We could assume that VINCOM is used like a conventional pseudo
> differential negative signal and have supply-vincom + non diffferential
> channels if that's the configuration wanted.
>
> Then for differential channels can support all the VINX VINX+1
> and swapped options.
> For VIN10 it gets fun as non differential and differential options
> I think map to same actual config. Don't see reason we need to express
> that in the binding though so let that have VIN10 VINCOM (probably using
> a magic channel number) and VIN10 pseudo differential.
>
> Similar setup for ADCIN15 equivalent usage
>
> Code wise this probably won't be particular hard to support in the driver
> (obviously I haven't tried though :) is it worth the effort to keep
> it inline with other devices that support pseudo differential channesl.
Then this would need to be done to any fully differential ADC as support
for pseudo differential channels is present (connect a fixed non 0 voltage
to the negative input).
The AD717x family supports pseudo differential channels as well... should
this change be applied to them too? It is just the case that the documentation
does not mentions this use case.
I think that a distinction needs to be made here:
- When a device is only pseudo differential, sure, it is in a different category
- When a device is fully differential and you are using it as pseudo-differential
you are having two inputs compared to one another
I would need more clarification is why would supply-vincom be a requirement.
The voltage supplied to VINCOM will not be used in any computation within
the driver. From the perspective of getting the data it doesn't matter if
you are using the channel in a pseudo-differential, single ended or fully
differential manner.
Regarding VINX VINX+1, it is not mandatory to respect those, from AD4111 page27:
"Due to the matching resistors on the analog front end, the
differential inputs must be paired together in the following
pairs: VIN0 and VIN1, VIN2 and VIN3, VIN4 and VIN5, and
VIN6 and VIN7. If any two voltage inputs are paired in a
configuration other than what is described in this data sheet,
the accuracy of the device cannot be guaranteed."
Tried the device and it works as fully differential when pairing any
VINx with VINCOM. Still works when selecting VINCOM as the positive
input of the ADC.
I really see this as overly complicated and unnecessary. These families
of ADCs are fully differential. If you are using it to measure a single ended
(Be it compared to 0V or pseudo differential where you are comparing to Vref/2
and obtaining results [Vref/2 , -Vref/2]) the final result will not require knowing
the common voltage.
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Ceclan, Dumitru @ 2024-04-09 8:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20240406152637.5d53c34f@jic23-huawei>
On 06/04/2024 17:26, Jonathan Cameron wrote:
> On Thu, 4 Apr 2024 16:08:56 +0300
> "Ceclan, Dumitru" <mitrutzceclan@gmail.com> wrote:
>
>> On 03/04/2024 18:22, David Lechner wrote:
>>> On Wed, Apr 3, 2024 at 2:50 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
>>>> On 02/04/2024 00:16, David Lechner wrote:
>>>>> On Mon, Apr 1, 2024 at 2:37 PM David Lechner <dlechner@baylibre.com> wrote:
>>>>>> On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
>>>>>> <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
>>>>>>> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>>>>>>>
>>>> ...
>>>>
>>>>>>> properties:
>>>>>>> reg:
>>>>>>> + description:
>>>>>>> + Reg values 16-19 are only permitted for ad4111/ad4112 current channels.
>>>>>>> minimum: 0
>>>>>>> - maximum: 15
>>>>>>> + maximum: 19
>>>>>> This looks wrong. Isn't reg describing the number of logical channels
>>>>>> (# of channel config registers)?
>>>>>>
>>>>>> After reviewing the driver, I see that > 16 is used as a way of
>>>>>> flagging current inputs, but still seems like the wrong way to do it.
>>>>>> See suggestion below.
>>>>>>
>>>>>>> diff-channels:
>>>>>>> + description:
>>>>>>> + For using current channels specify only the positive channel.
>>>>>>> + (IIN2+, IIN2−) -> diff-channels = <2 0>
>>>>>> I find this a bit confusing since 2 is already VIN2 and 0 is already
>>>>>> VIN0. I think it would make more sense to assign unique channel
>>>>>> numbers individually to the negative and positive current inputs.
>>>>>> Also, I think it makes sense to use the same numbers that the
>>>>>> registers in the datasheet use (8 - 11 for negative and 12 to 15 for
>>>>>> positive).
>>>>>>
>>>>>> So: (IIN2+, IIN2−) -> diff-channels = <13 10>
>>>>> Thinking about this a bit more...
>>>>>
>>>>> Since the current inputs have dedicated pins and aren't mix-and-match
>>>>> with multiple valid wiring configurations like the voltage inputs, do
>>>>> we even need to describe them in the devicetree?
>>>>>
>>>>> In the driver, the current channels would just be hard-coded like the
>>>>> temperature channel since there isn't any application-specific
>>>>> variation.
>>>> Sure, but we still need to offer the user a way to configure which
>>>> current inputs he wants and if they should use bipolar or unipolar coding.
>>> From the datasheet, it looks like only positive current input is
>>> allowed so I'm not sure bipolar applies here. But, yes, if there is
>>> some other variation in wiring or electrical signal that needs to be
>>> describe here, then it makes sense to allow a channel configuration
>>> node for it.
>>
>> AD4111 datasheet pg.29:
>> When the ADC is configured for bipolar operation, the output
>> code is offset binary with a negative full-scale voltage resulting
>> in a code of 000 … 000, a zero differential input voltage resulting in
>> a code of 100 … 000, and a positive full-scale input voltage
>> resulting in a code of 111 … 111. The output code for any
>> analog input voltage can be represented as
>> Code = 2^(N – 1) × ((V_IN × 0.1/V REF) + 1)
>> The output code for any input current is represented as
>> Code = 2^(N − 1) × ((I_IN × 50 Ω/V REF) + 1)
>>
>> I would say bipolar applies here, not a great idea because of the limitation on
>> the negative side (Input Current Range min:−0.5 max:+24 mA) so still, the option
>> is available.
> Just to check I am correct in thinking you 'might' use bipolar if you want
> to be able to measure small negative currents, but the range is much larger
> in the positive direction?
>
> J
Yes, exactly
^ permalink raw reply
* Re: [PATCH] arm64: dts: meson: fix S4 power-controller node
From: Xianwei Zhao @ 2024-04-09 8:15 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Kevin Hilman, Jerome Brunet, devicetree, linux-arm-kernel,
linux-amlogic, linux-kernel
In-Reply-To: <CAFBinCD=xWuhoX9cWcKU3bSGcsDShKbxnMVTdyfD84AFZQn8aw@mail.gmail.com>
Hi Martin,
Thanks for your review.
On 2024/4/9 01:27, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon, Apr 8, 2024 at 5:26 AM Xianwei Zhao via B4 Relay
> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
>>
>> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>>
>> The power-controller module works well by adding its parent
>> node secure-monitor.
>>
>
> Please add a Fixes tag here with the original commit where the
> incorrectly placed node was added.
Will add Fixes tag.
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> ---
>> arch/arm64/boot/dts/amlogic/meson-s4.dtsi | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
>> index ce90b35686a2..24d00dce4969 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
>> @@ -65,10 +65,13 @@ xtal: xtal-clk {
>> #clock-cells = <0>;
>> };
>>
>> - pwrc: power-controller {
>> - compatible = "amlogic,meson-s4-pwrc";
>> - #power-domain-cells = <1>;
>> - status = "okay";
>> + sm: secure-monitor {
>> + compatible = "amlogic,meson-gxbb-sm";
>> +
>> + pwrc: power-controller {
>> + compatible = "amlogic,meson-s4-pwrc";
>> + #power-domain-cells = <1>;
>> + };
> In Documentation/devicetree/bindings/firmware/amlogic,meson-gxbb-sm.yaml
> the hierarchy is:
> firmware {
> secure-monitor {
> power-controller {
> ...
> }
> }
> }
>
> Is this patch correct (and the documentation needs to be adapted) or
> is the documentation correct (and this patch has to be adapted)?
Will add firmware node to adapt documentation.
^ permalink raw reply
* [PATCH] dt-bindings: host1x: Add missing 'dma-coherent'
From: Jon Hunter @ 2024-04-09 8:23 UTC (permalink / raw)
To: Thierry Reding, Mikko Perttunen, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-tegra, devicetree, Jon Hunter
The dtbs_check reports that the 'dma-coherent' property is "unevaluated
and invalid" for the host1x@13e00000 device on Tegra194 and Tegra234
platforms. Fix this by updating the dt-binding document for host1x to
add the 'dma-coherent' property for these devices.
Fixes: 361238cdc525 ("arm64: tegra: Mark host1x as dma-coherent on Tegra194/234")
Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
index 94c5242c03b2..3563378a01af 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.yaml
@@ -177,6 +177,15 @@ allOf:
required:
- reg-names
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nvidia,tegra194-host1x
+ then:
+ properties:
+ dma-coherent: true
- if:
properties:
compatible:
@@ -226,6 +235,8 @@ allOf:
use. Should be a mapping of IDs 0..n to IOMMU entries corresponding to
usable stream IDs.
+ dma-coherent: true
+
required:
- reg-names
--
2.34.1
^ permalink raw reply related
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