* Re: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: kernel test robot @ 2024-04-06 10:42 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Rob Clark, Abhinav Kumar,
Dmitry Baryshkov, Sean Paul, Marijn Suijten, David Airlie,
Daniel Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: oe-kbuild-all, linux-arm-msm, linux-kernel, dri-devel, freedreno,
devicetree, Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-4-ce2b864251b1@linaro.org>
Hi Konrad,
kernel test robot noticed the following build errors:
[auto build test ERROR on 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd]
url: https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/soc-qcom-Move-some-socinfo-defines-to-the-header-expand-them/20240405-164231
base: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
patch link: https://lore.kernel.org/r/20240405-topic-smem_speedbin-v1-4-ce2b864251b1%40linaro.org
patch subject: [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
config: i386-buildonly-randconfig-003-20240406 (https://download.01.org/0day-ci/archive/20240406/202404061841.njUovDV7-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240406/202404061841.njUovDV7-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404061841.njUovDV7-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_read_speedbin':
>> drivers/gpu/drm/msm/adreno/adreno_gpu.c:1090:14: error: implicit declaration of function 'FIELD_PREP'; did you mean 'NEED_PGE'? [-Werror=implicit-function-declaration]
*speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
^~~~~~~~~~
NEED_PGE
cc1: some warnings being treated as errors
vim +1090 drivers/gpu/drm/msm/adreno/adreno_gpu.c
1062
1063 int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
1064 struct device *dev, u32 *speedbin)
1065 {
1066 u32 fcode, pcode;
1067 int ret;
1068
1069 /* Try reading the speedbin via a nvmem cell first */
1070 ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
1071 if (!ret && ret != -EINVAL)
1072 return ret;
1073
1074 ret = qcom_smem_get_feature_code(&fcode);
1075 if (ret) {
1076 dev_err(dev, "Couldn't get feature code from SMEM!\n");
1077 return ret;
1078 }
1079
1080 ret = qcom_smem_get_product_code(&pcode);
1081 if (ret) {
1082 dev_err(dev, "Couldn't get product code from SMEM!\n");
1083 return ret;
1084 }
1085
1086 /* Don't consider fcode for external feature codes */
1087 if (fcode <= SOCINFO_FC_EXT_RESERVE)
1088 fcode = SOCINFO_FC_UNKNOWN;
1089
> 1090 *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
1091 FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
1092
1093 return ret;
1094 }
1095
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 0/4] Introduce clock support for Airoha EN7581 SoC
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
this series is based on the following series:
https://patchwork.kernel.org/project/linux-arm-kernel/cover/cover.1709975956.git.lorenzo@kernel.org/
Changes since v1:
- add .enable()/disable() callbacks to pcie ops
- introduce en_clk_soc_data data structure in order to define multiple clk_ops
for each supported SoC
- rename clock node from system-controller to clock-controller
- add missing item descriptions in dt-binding
Lorenzo Bianconi (4):
dt-bindings: clock: airoha: add EN7581 binding
arm64: dts: airoha: Add EN7581 clock node
clk: en7523: Add en_clk_soc_data data structure
clk: en7523: Add EN7581 support
.../bindings/clock/airoha,en7523-scu.yaml | 31 ++-
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +
drivers/clk/clk-en7523.c | 190 ++++++++++++++++--
3 files changed, 211 insertions(+), 19 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH v2 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../bindings/clock/airoha,en7523-scu.yaml | 31 +++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index 79b0752faa91..3f4266637733 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -29,10 +29,13 @@ description: |
properties:
compatible:
items:
- - const: airoha,en7523-scu
+ - enum:
+ - airoha,en7523-scu
+ - airoha,en7581-scu
reg:
- maxItems: 2
+ minItems: 2
+ maxItems: 3
"#clock-cells":
description:
@@ -45,6 +48,30 @@ required:
- reg
- '#clock-cells'
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7523-scu
+ then:
+ properties:
+ reg:
+ items:
+ - description: scu base address
+ - description: misc scu base address
+
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7581-scu
+ then:
+ properties:
+ reg:
+ items:
+ - description: scu base address
+ - description: misc scu base address
+ - description: pb scu base address
+
additionalProperties: false
examples:
--
2.44.0
^ permalink raw reply related
* [PATCH v2 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
index 55eb1762fb11..5c4bfe3e1e5a 100644
--- a/arch/arm64/boot/dts/airoha/en7581.dtsi
+++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
@@ -2,6 +2,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/en7523-clk.h>
/ {
interrupt-parent = <&gic>;
@@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <1843200>;
};
+
+ scuclk: clock-controller@1fa20000 {
+ compatible = "airoha,en7581-scu";
+ reg = <0x0 0x1fa20000 0x0 0x400>,
+ <0x0 0x1fb00000 0x0 0x1000>,
+ <0x0 0x1fbe3400 0x0 0xfc>;
+ #clock-cells = <1>;
+ };
};
};
--
2.44.0
^ permalink raw reply related
* [PATCH v2 3/4] clk: en7523: Add en_clk_soc_data data structure
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce en_clk_soc_data data structure in order to define multiple
clk_ops for each supported SoC. This is a preliminary patch to
introduce EN7581 clock support.
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7cde328495e2..7eee921ab575 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -47,6 +47,10 @@ struct en_clk_gate {
struct clk_hw hw;
};
+struct en_clk_soc_data {
+ const struct clk_ops pcie_ops;
+};
+
static const u32 gsw_base[] = { 400000000, 500000000 };
static const u32 emi_base[] = { 333000000, 400000000 };
static const u32 bus_base[] = { 500000000, 540000000 };
@@ -145,11 +149,6 @@ static const struct en_clk_desc en7523_base_clks[] = {
}
};
-static const struct of_device_id of_match_clk_en7523[] = {
- { .compatible = "airoha,en7523-scu", },
- { /* sentinel */ }
-};
-
static unsigned int en7523_get_base_rate(void __iomem *base, unsigned int i)
{
const struct en_clk_desc *desc = &en7523_base_clks[i];
@@ -247,14 +246,10 @@ static void en7523_pci_unprepare(struct clk_hw *hw)
static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
void __iomem *np_base)
{
- static const struct clk_ops pcie_gate_ops = {
- .is_enabled = en7523_pci_is_enabled,
- .prepare = en7523_pci_prepare,
- .unprepare = en7523_pci_unprepare,
- };
+ const struct en_clk_soc_data *soc_data = of_device_get_match_data(dev);
struct clk_init_data init = {
.name = "pcie",
- .ops = &pcie_gate_ops,
+ .ops = &soc_data->pcie_ops,
};
struct en_clk_gate *cg;
@@ -264,7 +259,7 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
cg->base = np_base;
cg->hw.init = &init;
- en7523_pci_unprepare(&cg->hw);
+ init.ops->unprepare(&cg->hw);
if (clk_hw_register(dev, &cg->hw))
return NULL;
@@ -333,6 +328,19 @@ static int en7523_clk_probe(struct platform_device *pdev)
return r;
}
+static const struct en_clk_soc_data en7523_data = {
+ .pcie_ops = {
+ .is_enabled = en7523_pci_is_enabled,
+ .prepare = en7523_pci_prepare,
+ .unprepare = en7523_pci_unprepare,
+ },
+};
+
+static const struct of_device_id of_match_clk_en7523[] = {
+ { .compatible = "airoha,en7523-scu", .data = &en7523_data },
+ { /* sentinel */ }
+};
+
static struct platform_driver clk_en7523_drv = {
.probe = en7523_clk_probe,
.driver = {
--
2.44.0
^ permalink raw reply related
* [PATCH v2 4/4] clk: en7523: Add EN7581 support
From: Lorenzo Bianconi @ 2024-04-06 10:43 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712399980.git.lorenzo@kernel.org>
Introduce EN7581 clock support to clk-en7523 driver.
Add hw_init callback to en_clk_soc_data data structure.
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 158 +++++++++++++++++++++++++++++++++++++--
1 file changed, 153 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7eee921ab575..381605be333f 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -4,13 +4,16 @@
#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <dt-bindings/clock/en7523-clk.h>
#define REG_PCI_CONTROL 0x88
#define REG_PCI_CONTROL_PERSTOUT BIT(29)
#define REG_PCI_CONTROL_PERSTOUT1 BIT(26)
+#define REG_PCI_CONTROL_REFCLK_EN0 BIT(23)
#define REG_PCI_CONTROL_REFCLK_EN1 BIT(22)
+#define REG_PCI_CONTROL_PERSTOUT2 BIT(16)
#define REG_GSW_CLK_DIV_SEL 0x1b4
#define REG_EMI_CLK_DIV_SEL 0x1b8
#define REG_BUS_CLK_DIV_SEL 0x1bc
@@ -18,10 +21,25 @@
#define REG_SPI_CLK_FREQ_SEL 0x1c8
#define REG_NPU_CLK_DIV_SEL 0x1fc
#define REG_CRYPTO_CLKSRC 0x200
-#define REG_RESET_CONTROL 0x834
+#define REG_RESET_CONTROL2 0x830
+#define REG_RESET2_CONTROL_PCIE2 BIT(27)
+#define REG_RESET_CONTROL1 0x834
#define REG_RESET_CONTROL_PCIEHB BIT(29)
#define REG_RESET_CONTROL_PCIE1 BIT(27)
#define REG_RESET_CONTROL_PCIE2 BIT(26)
+/* EN7581 */
+#define REG_PCIE0_MEM 0x00
+#define REG_PCIE0_MEM_MASK 0x04
+#define REG_PCIE1_MEM 0x08
+#define REG_PCIE1_MEM_MASK 0x0c
+#define REG_PCIE2_MEM 0x10
+#define REG_PCIE2_MEM_MASK 0x14
+#define REG_PCIE_RESET_OPEN_DRAIN 0x018c
+#define REG_PCIE_RESET_OPEN_DRAIN_MASK GENMASK(2, 0)
+#define REG_NP_SCU_PCIC 0x88
+#define REG_NP_SCU_SSTR 0x9c
+#define REG_PCIE_XSI0_SEL_MASK GENMASK(14, 13)
+#define REG_PCIE_XSI1_SEL_MASK GENMASK(12, 11)
struct en_clk_desc {
int id;
@@ -49,6 +67,8 @@ struct en_clk_gate {
struct en_clk_soc_data {
const struct clk_ops pcie_ops;
+ int (*hw_init)(struct platform_device *pdev, void __iomem *base,
+ void __iomem *np_base);
};
static const u32 gsw_base[] = { 400000000, 500000000 };
@@ -211,14 +231,14 @@ static int en7523_pci_prepare(struct clk_hw *hw)
usleep_range(1000, 2000);
/* Reset to default */
- val = readl(np_base + REG_RESET_CONTROL);
+ val = readl(np_base + REG_RESET_CONTROL1);
mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
REG_RESET_CONTROL_PCIEHB;
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(1000, 2000);
- writel(val | mask, np_base + REG_RESET_CONTROL);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
msleep(100);
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(5000, 10000);
/* Release device */
@@ -259,6 +279,9 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
cg->base = np_base;
cg->hw.init = &init;
+
+ if (init.ops->disable)
+ init.ops->disable(&cg->hw);
init.ops->unprepare(&cg->hw);
if (clk_hw_register(dev, &cg->hw))
@@ -267,6 +290,111 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
return &cg->hw;
}
+static int en7581_pci_is_enabled(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
+ val = readl(cg->base + REG_PCI_CONTROL);
+ return (val & mask) == mask;
+}
+
+static int en7581_pci_prepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ usleep_range(5000, 10000);
+
+ return 0;
+}
+
+static int en7581_pci_enable(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val | mask, np_base + REG_PCI_CONTROL);
+ msleep(250);
+
+ return 0;
+}
+
+static void en7581_pci_unprepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ msleep(100);
+}
+
+static void en7581_pci_disable(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val & ~mask, np_base + REG_PCI_CONTROL);
+ usleep_range(1000, 2000);
+}
+
+static int en7581_clk_hw_init(struct platform_device *pdev,
+ void __iomem *base,
+ void __iomem *np_base)
+{
+ void __iomem *pb_base;
+ u32 val;
+
+ pb_base = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(pb_base))
+ return PTR_ERR(pb_base);
+
+ val = readl(np_base + REG_NP_SCU_SSTR);
+ val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
+ writel(val, np_base + REG_NP_SCU_SSTR);
+ val = readl(np_base + REG_NP_SCU_PCIC);
+ writel(val | 3, np_base + REG_NP_SCU_PCIC);
+
+ writel(0x20000000, pb_base + REG_PCIE0_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE0_MEM_MASK);
+ writel(0x24000000, pb_base + REG_PCIE1_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE1_MEM_MASK);
+ writel(0x28000000, pb_base + REG_PCIE2_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);
+
+ val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
+ writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
+ base + REG_PCIE_RESET_OPEN_DRAIN);
+
+ return 0;
+}
+
static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
void __iomem *base, void __iomem *np_base)
{
@@ -299,6 +427,7 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
static int en7523_clk_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
+ const struct en_clk_soc_data *soc_data;
struct clk_hw_onecell_data *clk_data;
void __iomem *base, *np_base;
int r;
@@ -311,6 +440,13 @@ static int en7523_clk_probe(struct platform_device *pdev)
if (IS_ERR(np_base))
return PTR_ERR(np_base);
+ soc_data = of_device_get_match_data(&pdev->dev);
+ if (soc_data->hw_init) {
+ r = soc_data->hw_init(pdev, base, np_base);
+ if (r)
+ return r;
+ }
+
clk_data = devm_kzalloc(&pdev->dev,
struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
GFP_KERNEL);
@@ -336,8 +472,20 @@ static const struct en_clk_soc_data en7523_data = {
},
};
+static const struct en_clk_soc_data en7581_data = {
+ .pcie_ops = {
+ .is_enabled = en7581_pci_is_enabled,
+ .prepare = en7581_pci_prepare,
+ .enable = en7581_pci_enable,
+ .unprepare = en7581_pci_unprepare,
+ .disable = en7581_pci_disable,
+ },
+ .hw_init = en7581_clk_hw_init,
+};
+
static const struct of_device_id of_match_clk_en7523[] = {
{ .compatible = "airoha,en7523-scu", .data = &en7523_data },
+ { .compatible = "airoha,en7581-scu", .data = &en7581_data },
{ /* sentinel */ }
};
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Krzysztof Kozlowski @ 2024-04-06 10:57 UTC (permalink / raw)
To: Peng Fan (OSS), Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-1-9fc9186856c2@nxp.com>
On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> When adding vendor extension protocols, there is dt-schema warning:
> "
> imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not match any
> of the regexes: 'pinctrl-[0-9]+'
> "
>
> Set additionalProperties to true to address the issue.
I do not see anything addressed here, except making the binding
accepting anything anywhere...
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Krzysztof Kozlowski @ 2024-04-06 11:02 UTC (permalink / raw)
To: Peng Fan (OSS), Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-2-9fc9186856c2@nxp.com>
On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add i.MX SCMI Extension protocols bindings for:
> - Battery Backed Secure Module(BBSM)
Which is what?
> - MISC settings such as General Purpose Registers settings.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> .../devicetree/bindings/firmware/imx,scmi.yaml | 80 ++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> new file mode 100644
> index 000000000000..7ee19a661d83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/imx,scmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX System Control and Management Interface(SCMI) Vendor Protocols Extension
> +
> +maintainers:
> + - Peng Fan <peng.fan@nxp.com>
> +
> +allOf:
> + - $ref: arm,scmi.yaml#
Sorry, but arm,scmi is a final schema. Is your plan to define some
common part?
> +
> +properties:
> + protocol@81:
> + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> + unevaluatedProperties: false
> + description:
> + The BBM Protocol is for managing Battery Backed Secure Module (BBSM) RTC
> + and the ON/OFF Key
> +
> + properties:
> + reg:
> + const: 0x81
> +
> + required:
> + - reg
> +
> + protocol@84:
> + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> + unevaluatedProperties: false
> + description:
> + The MISC Protocol is for managing SoC Misc settings, such as GPR settings
Genera register is not a setting... this is a pleonasm. Please be more
specific what is the GPR, MISC protocol etc.
> +
> + properties:
> + reg:
> + const: 0x84
> +
> + wakeup-sources:
> + description:
> + Each entry consists of 2 integers, represents the source and electric signal edge
Can you answer questions from reviewers?
> + items:
> + items:
> + - description: the wakeup source
> + - description: the wakeup electric signal edge
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +
> + required:
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + firmware {
> + scmi {
> + compatible = "arm,scmi";
> + mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
> + shmem = <&scmi_buf0>, <&scmi_buf1>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + protocol@81 {
> + reg = <0x81>;
> + };
> +
> + protocol@84 {
> + reg = <0x84>;
> + wakeup-sources = <0x8000 1
> + 0x8001 1
> + 0x8002 1
> + 0x8003 1
> + 0x8004 1>;
Nothing improved... If you are going to ignore reviews, then you will
only get NAKed.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 0/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming
Some Grand Prime use a Bosch BMC150 accelerometer/magnetometer combo.
The chip provides two separate I2C devices for the accelerometer
and magnetometer that are already supported by the bmc150-accel
and bmc150-magn driver.
Some Grand Prime use a ST LSM303C accelerometer/magnetometer combo.
Core Prime LTE uses ST LIS2HH12 accelerometer.
Add support for them.
---
v2: fix accelerometer@1d: 'interrupt-names' does not match any of the regexes:
'pinctrl-[0-9]+'
^ permalink raw reply
* [PATCH v2 1/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add BMC150 accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming, Joe Mason
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
From: Joe Mason <buddyjojo06@outlook.com>
Some Grand Prime use a Bosch BMC150 accelerometer/magnetometer combo.
The chip provides two separate I2C devices for the accelerometer
and magnetometer that are already supported by the bmc150-accel
and bmc150-magn driver.
Signed-off-by: Joe Mason <buddyjojo06@outlook.com>
[Stephan: Move sensors to common dtsi, disabled by default]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Raymond: Add it to grandprimelte. Use interrupts-extended]
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../qcom/msm8916-samsung-fortuna-common.dtsi | 44 +++++++++++++++++++
.../dts/qcom/msm8916-samsung-gprimeltecan.dts | 8 ++++
.../qcom/msm8916-samsung-grandprimelte.dts | 8 ++++
3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
index 5e933fb8b363..b6e1fe8b0056 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
@@ -117,6 +117,43 @@ muic: extcon@25 {
};
};
+&blsp_i2c2 {
+ /* Available sensors vary depending on model variant */
+ status = "okay";
+
+ bosch_accel: accelerometer@10 {
+ compatible = "bosch,bmc150_accel";
+ reg = <0x10>;
+ interrupts-extended = <&tlmm 115 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <&pm8916_l5>;
+ vddio-supply = <&pm8916_l5>;
+
+ pinctrl-0 = <&accel_int_default>;
+ pinctrl-names = "default";
+
+ mount-matrix = "0", "-1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
+
+ status = "disabled";
+ };
+
+ bosch_magn: magnetometer@12 {
+ compatible = "bosch,bmc150_magn";
+ reg = <0x12>;
+
+ vdd-supply = <&pm8916_l5>;
+ vddio-supply = <&pm8916_l5>;
+
+ mount-matrix = "0", "-1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
+
+ status = "disabled";
+ };
+};
+
&blsp_i2c4 {
status = "okay";
@@ -223,6 +260,13 @@ &wcnss_mem {
};
&tlmm {
+ accel_int_default: accel-int-default-state {
+ pins = "gpio115";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
backlight_en_default: backlight-en-default-state {
pins = "gpio98";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
index 9d65fa58ba92..4dc74e8bf1d8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
@@ -21,6 +21,14 @@ tz-apps@85500000 {
};
};
+&bosch_accel {
+ status = "okay";
+};
+
+&bosch_magn {
+ status = "okay";
+};
+
&mpss_mem {
/* Firmware for gprimeltecan needs more space */
reg = <0x0 0x86800000 0x0 0x5400000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
index a66ce4b13547..cffad734c4df 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
@@ -10,6 +10,14 @@ / {
chassis-type = "handset";
};
+&bosch_accel {
+ status = "okay";
+};
+
+&bosch_magn {
+ status = "okay";
+};
+
&mpss_mem {
/* Firmware for grandprimelte needs more space */
reg = <0x0 0x86800000 0x0 0x5400000>;
--
2.39.2
^ permalink raw reply related
* [PATCH v2 2/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add LSM303C accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming, Siddharth Manthan
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
From: Siddharth Manthan <siddharth.manthan@gmail.com>
Some Grand Prime use a ST LSM303C accelerometer/magnetometer combo.
Add support for it.
Signed-off-by: Siddharth Manthan <siddharth.manthan@gmail.com>
[Stephan: Move sensors to common dtsi (disabled by default)]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Raymond: Use interrupts-extended]
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../dts/qcom/msm8216-samsung-fortuna3g.dts | 8 +++++
.../qcom/msm8916-samsung-fortuna-common.dtsi | 33 +++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
index 366914be7d53..e7f6df229f9a 100644
--- a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
+++ b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
@@ -9,3 +9,11 @@ / {
compatible = "samsung,fortuna3g", "qcom,msm8916";
chassis-type = "handset";
};
+
+&st_accel {
+ status = "okay";
+};
+
+&st_magn {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
index b6e1fe8b0056..17d21949dfec 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
@@ -152,6 +152,39 @@ bosch_magn: magnetometer@12 {
status = "disabled";
};
+
+ st_accel: accelerometer@1d {
+ compatible = "st,lsm303c-accel";
+ reg = <0x1d>;
+ interrupts-extended = <&tlmm 115 IRQ_TYPE_LEVEL_HIGH>;
+
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l5>;
+
+ pinctrl-0 = <&accel_int_default>;
+ pinctrl-names = "default";
+
+ st,drdy-int-pin = <1>;
+ mount-matrix = "0", "-1", "0",
+ "1", "0", "0",
+ "0", "0", "-1";
+
+ status = "disabled";
+ };
+
+ st_magn: magnetometer@1e {
+ compatible = "st,lsm303c-magn";
+ reg = <0x1e>;
+
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l5>;
+
+ mount-matrix = "0", "-1", "0",
+ "1", "0", "0",
+ "0", "0", "-1";
+
+ status = "disabled";
+ };
};
&blsp_i2c4 {
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v19 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Krzysztof Kozlowski @ 2024-04-06 11:14 UTC (permalink / raw)
To: Dmitry Baryshkov, Bjorn Andersson
Cc: Greg Kroah-Hartman, Johan Hovold, Krishna Kurapati,
Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Conor Dooley, Thinh Nguyen, Felipe Balbi,
devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Johan Hovold
In-Reply-To: <CAA8EJpr94_uCHoJyQd2uQyZ35xXixPPMHPVGXciZn+GftUUjnA@mail.gmail.com>
On 05/04/2024 22:36, Dmitry Baryshkov wrote:
>>>>>
>>>>> Also get those internal reviewers to sign-off on the commits and have
>>>>> that show up when you post them next. That way they are also
>>>>> responsible for this patchset, it's not fair that they are making you do
>>>>> all the work here :)
>>>>>
>>>>
>>>> I like this idea and I'm open to us changing our way of handling this.
>>>>
>>>> But unless such internal review brings significant input to the
>>>> development I'd say a s-o-b would take the credit from the actual
>>>> author.
>>>
>>> It does not do that at all. It provides proof that someone else has
>>> reviewed it and agrees with it. Think of it as a "path of blame" for
>>> when things go bad (i.e. there is a bug in the submission.) Putting
>>> your name on it makes you take responsibility if that happens.
>>>
>>
>> Right, this is why I like your idea.
>>
>> But as s-o-b either builds a trail of who handled the patch, or reflects
>> that it was co-authored by multiple people, I don't think either one
>> properly reflects reality.
>>
>>>> We've discussed a few times about carrying Reviewed-by et al from the
>>>> internal reviews, but as maintainer I dislike this because I'd have no
>>>> way to know if a r-b on vN means the patch was reviewed, or if it was
>>>> just "accidentally" carried from v(N-1).
>>>> But it might be worth this risk, is this something you think would be
>>>> appropriate?
>>>
>>> For some companies we REQUIRE this to happen due to low-quality
>>> submissions and waste of reviewer's time. Based on the track record
>>> here for some of these patchsets, hopefully it doesn't become a
>>> requirement for this company as well :)
>>>
>>
>> Interesting, I was under the impression that we (maintainers) didn't
>> want such internally originating tags.
>
> But why? It just means that the patch has been reviewed. In some rare
> cases we explicitly ask a developer to have all the patches reviewed
> before sending them upstream. In such a case having an R-B tag
> fulfills the expectation of the maintainer: it shows that another
> engineer has reviewed the patch.
Wait, there are two types of internal reviews.
Automatic, +1 from Gerrit or from whatever internal processes require,
which are not useful because these internal reviewers do not actually
review. I have seen a lot of such and I complain. It's easy to spot them
- a patchset consisting of few patches, including trivial ones, all of
them carrying one more more review tags. Even fixing a typo: reviewed
tag. Plus then you see that quality of the patchset is actually poor.
Another are real reviews done internally. If they are real, I find them
useful.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: qcom: msm8916-samsung-rossa: Add LIS2HH12 accelerometer
From: Raymond Hackley @ 2024-04-06 11:15 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
Core Prime LTE uses ST LIS2HH12 accelerometer. Add support for it.
[Stephen: Use common &st_accel definition from common dtsi]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
index b438fa81886c..db95bdbb9f32 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
@@ -26,3 +26,11 @@ &clk_pwm {
&clk_pwm_backlight {
status = "disabled";
};
+
+&st_accel {
+ compatible = "st,lis2hh12";
+ mount-matrix = "1", "0", "0",
+ "0", "-1", "0",
+ "0", "0", "1";
+ status = "okay";
+};
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] dt-bindings: extcon: ptn5150: Document the 'port' node
From: Krzysztof Kozlowski @ 2024-04-06 11:26 UTC (permalink / raw)
To: Fabio Estevam
Cc: cw00.choi, myungjoo.ham, robh, conor+dt, devicetree, marex,
Fabio Estevam
In-Reply-To: <CAOMZO5A7yiYdeOjv6BabDSNKk4Gp=n1qwJP0seFvmjYYgjvhOA@mail.gmail.com>
On 05/04/2024 21:58, Fabio Estevam wrote:
> On Fri, Apr 5, 2024 at 6:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> But more important, what about USB C connector?
>
> In a previous attempt, I tried describing the USB connector:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.9-rc2&id=a4dca89fe8a1585af73e362f5f4e3189a00abf8e
>
> This broke the USB support as reported by Marek, so I had to revert it:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.9-rc2&id=f954785a124e77d4e6bb52cab689a8de447999aa
>
> Would it be OK if I send a v2 without the USB C connector description
> and address your other comments?
No, because I think this should be the connector. Look at datasheet of
ptn5150 and ptn5110. Aren't both describing similar hardware?
Instead adding some sort of hacked-hardware-representation, please
investigate why your previous commit broke things.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 02/11] spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically
From: Krzysztof Kozlowski @ 2024-04-06 11:38 UTC (permalink / raw)
To: Théo Lebrun, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vaishnav Achath, Thomas Bogendoerfer, Rob Herring
Cc: linux-spi, devicetree, linux-kernel, linux-mips,
Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
Tawfik Bayouk
In-Reply-To: <20240405-cdns-qspi-mbly-v2-2-956679866d6d@bootlin.com>
On 05/04/2024 17:02, Théo Lebrun wrote:
> Compatibles are ordered by date of addition.
> Switch to (deterministic) alphabetical ordering.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Krzysztof Kozlowski @ 2024-04-06 11:50 UTC (permalink / raw)
To: Lorenzo Bianconi, linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <99734deb28889e685a764da94418f68b55ee3bdc.1712399981.git.lorenzo@kernel.org>
On 06/04/2024 12:43, Lorenzo Bianconi wrote:
> Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../bindings/clock/airoha,en7523-scu.yaml | 31 +++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index 79b0752faa91..3f4266637733 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -29,10 +29,13 @@ description: |
> properties:
> compatible:
> items:
> - - const: airoha,en7523-scu
> + - enum:
> + - airoha,en7523-scu
> + - airoha,en7581-scu
>
> reg:
> - maxItems: 2
> + minItems: 2
> + maxItems: 3
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2] dt-bindings: ti,pcm1681: Convert to dtschema
From: Animesh Agarwal @ 2024-04-06 14:14 UTC (permalink / raw)
Cc: animeshagarwal28, Shenghao Ding, Kevin Lu, Baojun Xu,
Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
Convert the Texas Instruments PCM1681 bindings to DT schema.
Make bindings complete by adding #sound-dai-cells.
Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
---
Changes in v2:
- Added existing maintainers to the list of maintainers.
- Added #sound-dai-cells, $ref to dai-common and unevaluatedProperties: false.
- Added "audio-codec" in example.
---
.../devicetree/bindings/sound/ti,pcm1681.txt | 15 -------
.../devicetree/bindings/sound/ti,pcm1681.yaml | 43 +++++++++++++++++++
2 files changed, 43 insertions(+), 15 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.txt
create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt b/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
deleted file mode 100644
index 4df17185ab80..000000000000
--- a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-Texas Instruments PCM1681 8-channel PWM Processor
-
-Required properties:
-
- - compatible: Should contain "ti,pcm1681".
- - reg: The i2c address. Should contain <0x4c>.
-
-Examples:
-
- i2c_bus {
- pcm1681@4c {
- compatible = "ti,pcm1681";
- reg = <0x4c>;
- };
- };
diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
new file mode 100644
index 000000000000..5aa00617291c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,pcm1681.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments PCM1681 8-channel PWM Processor
+
+maintainers:
+ - Shenghao Ding <shenghao-ding@ti.com>
+ - Kevin Lu <kevin-lu@ti.com>
+ - Baojun Xu <baojun.xu@ti.com>
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ const: ti,pcm1681
+
+ reg:
+ maxItems: 1
+
+ "#sound-dai-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pcm1681: audio-codec@4c {
+ compatible = "ti,pcm1681";
+ reg = <0x4c>;
+ };
+ };
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Jonathan Cameron @ 2024-04-06 14:26 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: David Lechner, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <aa84a3c5-a3e6-4c76-9b67-624ed8d8c704@gmail.com>
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
>
^ permalink raw reply
* Re: [PATCH v2 0/3] Add support for the IPQ5321 SoC
From: Kathiravan Thirumoorthy @ 2024-04-06 14:48 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Ilia Lin, Rafael J. Wysocki, Viresh Kumar
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pm,
Krzysztof Kozlowski, Mukesh Ojha
In-Reply-To: <20240325-ipq5321-sku-support-v2-0-f30ce244732f@quicinc.com>
On 3/25/2024 9:19 PM, Kathiravan Thirumoorthy wrote:
> IPQ5321 SoC belong to IPQ5332 family. Add the SoC ID and the cpufreq
> support. Maximum cpufreq for IPQ5321 is 1.1GHZ, which is determined
> based on the eFuse.
>
> Viresh is okay to merge the cpufreq change via qcom tree[1] and provided
> his Ack.
>
> [1]
> https://lore.kernel.org/linux-arm-msm/20240306053200.6iwrviltwt3pnfnt@vireshk-i7/
Gentle Reminder...
>
> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> ---
> Changes in v2:
> - rebase on next-20240325
> - pick up the tags
> - Link to v1:
> https://lore.kernel.org/linux-arm-msm/20240228-ipq5321-sku-support-v1-0-14e4d4715f4b@quicinc.com/
>
> ---
> Kathiravan Thirumoorthy (3):
> dt-bindings: arm: qcom,ids: Add SoC ID for IPQ5321
> soc: qcom: socinfo: Add SoC ID for IPQ5321
> cpufreq: qcom-nvmem: add support for IPQ5321
>
> drivers/cpufreq/qcom-cpufreq-nvmem.c | 1 +
> drivers/soc/qcom/socinfo.c | 1 +
> include/dt-bindings/arm/qcom,ids.h | 1 +
> 3 files changed, 3 insertions(+)
> ---
> base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
> change-id: 20240228-ipq5321-sku-support-bd07056d5e01
>
> Best regards,
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Jonathan Cameron @ 2024-04-06 14:53 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <CAMknhBHu8DveBgV3cor8RP2Up4Zs-+QRx7S2aoHZ_3iKiErVjg@mail.gmail.com>
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.
> > 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?
>
> >
> > >> +
> > >> + 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.
>
> > >> items:
> > >> minimum: 0
> > >> maximum: 31
> > >> @@ -166,7 +191,6 @@ allOf:
> > >> - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > >>
> > >> # Only ad7172-4, ad7173-8 and ad7175-8 support vref2
> > >> - # Other models have [0-3] channel registers
> > >
> > > Did you forget to remove
> > >
> > > reg:
> > > maximum: 3
> > >
> > > from this if statement that this comment is referring to?
> > >
> > >
> >
> >
> > Other way around, forgot in a previous patch to remove the comment.
> > I'll move this change to a precursor patch.
> >
> > >> - if:
> > >> properties:
> > >> compatible:
> > >> @@ -187,6 +211,37 @@ allOf:
> > >> - vref
> > >> - refout-avss
> > >> - avdd
> > >> +
> > >> + - if:
> > >> + properties:
> > >> + compatible:
> > >> + contains:
> > >> + enum:
> > >> + - adi,ad4114
> > >> + - adi,ad4115
> > >> + - adi,ad4116
> > >> + - adi,ad7173-8
> > >> + - adi,ad7175-8
> > >> + then:
> > >> + patternProperties:
> > >> + "^channel@[0-9a-f]$":
> > >> + properties:
> > >> + reg:
> > >> + maximum: 15
> > >
> > > As with the previous reg comment, this if statement should not be
> > > needed since maximum should not be changed to 19.
> > >
> >
> > We'll see what is the best approach regarding the current channels,
> > perhaps the one you mentioned in the later reply with always configuring
> > like the temp channel.
> >
That's an option as well. In many early drivers we just provided all the
channels. Somewhat depends on whether people buy devices with lots of
channels they don't wire. Mostly I suspect they don't as that's money
wasted!
Jonathan
^ permalink raw reply
* Re: [PATCH 2/6] iio: adc: ad7173: fix buffers enablement for ad7176-2
From: Jonathan Cameron @ 2024-04-06 14:56 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
In-Reply-To: <20240401-ad4111-v1-2-34618a9cc502@analog.com>
On Mon, 01 Apr 2024 18:32:20 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> AD7176-2 does not feature input buffers, enable buffers only on
> supported models.
>
> Fixes: cff259bf7274 ("iio: adc: ad7173: fix buffers enablement for ad7176-2")
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
How bad is this? If you can find out if writing those bits does anything
harmful (they are reserved and datasheet says should be written 0 I think)
That will help people decide whether to backport the fix?
> ---
> drivers/iio/adc/ad7173.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index f6d29abe1d04..8a95b1391826 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -145,6 +145,7 @@ struct ad7173_device_info {
> unsigned int id;
> char *name;
> bool has_temp;
> + bool has_input_buf;
> bool has_int_ref;
> bool has_ref2;
> u8 num_gpios;
> @@ -212,6 +213,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -224,6 +226,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = false,
> + .has_input_buf = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -237,6 +240,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> @@ -251,6 +255,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7175_sinc5_data_rates,
> @@ -263,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 16 * HZ_PER_MHZ,
> @@ -289,6 +295,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .odr_start_value = AD7177_ODR_START_VALUE,
> @@ -932,7 +939,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> AD7173_CH_ADDRESS(chan_arr[chan_index].channel,
> chan_arr[chan_index].channel2);
> chan_st_priv->cfg.bipolar = false;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>
> @@ -989,7 +996,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>
> chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
> chan_st_priv->chan_reg = chan_index;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.odr = 0;
>
> chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
>
^ permalink raw reply
* Re: [PATCH 4/6] iio: adc: ad7173: refactor ain and vref selection
From: Jonathan Cameron @ 2024-04-06 15:03 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
In-Reply-To: <20240401-ad4111-v1-4-34618a9cc502@analog.com>
On Mon, 01 Apr 2024 18:32:22 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> Move validation of analog inputs and reference voltage selection to
> separate functions.
>
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
A few line wrapping comments inline.
> ---
> drivers/iio/adc/ad7173.c | 59 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 699bc6970790..bf5a5b384fe2 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -910,6 +910,41 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
> &st->int_clk_hw);
> }
>
> +static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> + unsigned int ain[2])
> +{
> + struct device *dev = &st->sd.spi->dev;
> +
> + if (ain[0] >= st->info->num_inputs ||
> + ain[1] >= st->info->num_inputs)
No need to line wrap the above - its under 80 chars on one line with the
new reduced indent due to factoring this out.
> + return dev_err_probe(dev, -EINVAL,
> + "Input pin number out of range for pair (%d %d).\n",
> + ain[0], ain[1]);
> +
> + return 0;
> +}
> +
> +static int ad7173_validate_reference(struct ad7173_state *st, int ref_sel)
> +{
> + struct device *dev = &st->sd.spi->dev;
> + int ret;
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF && !st->info->has_int_ref)
> + return dev_err_probe(dev, -EINVAL,
> + "Internal reference is not available on current model.\n");
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> + return dev_err_probe(dev, -EINVAL,
> + "External reference 2 is not available on current model.\n");
> +
> + ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "Cannot use reference %u\n", ref_sel);
Can pull the string to previous line and then align ref_sel just after (
whilst still remaining under 80 chars and end up a little prettier.
> +
> + return 0;
> +}
> +
> static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> {
> struct ad7173_channel *chans_st_arr, *chan_st_priv;
> @@ -970,11 +1005,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> if (ret)
> return ret;
>
> - if (ain[0] >= st->info->num_inputs ||
> - ain[1] >= st->info->num_inputs)
> - return dev_err_probe(dev, -EINVAL,
> - "Input pin number out of range for pair (%d %d).\n",
> - ain[0], ain[1]);
> + ret = ad7173_validate_voltage_ain_inputs(st, ain);
> + if (ret)
> + return ret;
>
> ret = fwnode_property_match_property_string(child,
> "adi,reference-select",
> @@ -985,19 +1018,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> else
> ref_sel = ret;
>
> - if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF &&
> - !st->info->has_int_ref)
> - return dev_err_probe(dev, -EINVAL,
> - "Internal reference is not available on current model.\n");
> -
> - if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> - return dev_err_probe(dev, -EINVAL,
> - "External reference 2 is not available on current model.\n");
> -
> - ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> - if (ret < 0)
> - return dev_err_probe(dev, ret,
> - "Cannot use reference %u\n", ref_sel);
> + ret = ad7173_validate_reference(st, ref_sel);
> + if (ret)
> + return ret;
>
> if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>
^ permalink raw reply
* Re: [PATCH 6/6] iio: adc: ad7173: Add support for AD411x devices
From: Jonathan Cameron @ 2024-04-06 15:10 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <CAMknhBFZKKim4JyXLpHY=EoyW4ZPp03aHte5xy0ZkDnW16sfeA@mail.gmail.com>
> >
> > >> + case IIO_CURRENT:
> > >> + *val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
> > >> + *val /= AD4111_SHUNT_RESISTOR_OHM;
> > >> + *val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
> > >
> > > Static analysis tools like to complain about using bool as int.
> > > Probably more clear to write it as (ch->cfg.bipolar ? 1 : 0) anyway.
> > >
> > Maybe it does not apply here, but i followed this advice:
> >
> > Andy Shevchenko V1 of AD7173 (named initially ad717x)
> > "
> > > + return (bool)(value & mask);
> >
> > This is weird. You have int which you get from bool, wouldn't be better
> > to use
> > !!(...) as other GPIO drivers do?
>
> As long as the build bots don't complain, there isn't a reason to
> change it. It is just a matter of personal preference at that point.
>
> I got a sparse warning for something like this recently [1], but maybe
> that case was just because it was inside of a FIELD_PREP() using it as
> bit logic instead of addition and we won't get any warnings here.
>
> [1]: https://lore.kernel.org/linux-iio/20240129195611.701611-3-dlechner@baylibre.com/
It was common to use !! for a number of years, but then it got a
comment from Linus Torvalds making reasonable point that it isn't
easy to read, so slight preference these days is for a ternary.
^ permalink raw reply
* Re: [PATCH v7 0/8] iio: accel: adxl345: Add spi-3wire feature
From: Jonathan Cameron @ 2024-04-06 15:19 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240401194906.56810-1-l.rubusch@gmail.com>
On Mon, 1 Apr 2024 19:48:58 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:
> Pass a function setup() as pointer from SPI/I2C specific modules to the
> core module. Implement setup() to pass the spi-3wire bus option, if
> declared in the device-tree.
>
> In the core module then update data_format register configuration bits
> instead of overwriting it. The changes allow to remove a data_range field.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Series applied to the togreg branch of iio.git and pushed out initially
as testing to let 0-day work it's magic (find build issues for us!)
I'll push it out for linux-next to pick up sometime in next week (ish)
Jonathan
> ---
> V1 -> V2: Split into spi-3wire and refactoring
> V2 -> V3: Split further, focus on needed changesets
> V3 -> V4: Drop "Remove single info instances";
> split "Group bus configuration" into separat
> comment patch; reorder patch set
> V4 -> V5: Refrase comments; Align comments to 75; rebuild FORMAT_MASK by
> available flags; fix indention
> V5 -> V6: Remove FORMAT_MASK by a local variable on call site;
> Refrase comments;
> Remove unneeded include
> V6 -> V7: Restructure optional passing the setup() to core's probe()
> Guarantee that initially a regmap_write() was called to init
> all bits to a defined state
> - When a setup() e.g. for 3wire is passed, then call
> regmap_write() inside the setup(). In the following
> core's probe() has to call regmap_update()
> - When NULL is passed, then call regmap_write() in core's
> probe()
> - Refactoring: remove obvious comments and simplify code
>
> Lothar Rubusch (8):
> iio: accel: adxl345: Make data_range obsolete
> iio: accel: adxl345: Group bus configuration
> iio: accel: adxl345: Move defines to header
> dt-bindings: iio: accel: adxl345: Add spi-3wire
> iio: accel: adxl345: Pass function pointer to core
> iio: accel: adxl345: Reorder probe initialization
> iio: accel: adxl345: Add comment to probe
> iio: accel: adxl345: Add spi-3wire option
>
> .../bindings/iio/accel/adi,adxl345.yaml | 2 +
> drivers/iio/accel/adxl345.h | 36 +++++++-
> drivers/iio/accel/adxl345_core.c | 92 ++++++++++---------
> drivers/iio/accel/adxl345_i2c.c | 2 +-
> drivers/iio/accel/adxl345_spi.c | 10 +-
> 5 files changed, 94 insertions(+), 48 deletions(-)
>
^ permalink raw reply
* [PATCH] ARM: dts: qcom: msm8974-sony-shinano: Enable vibrator
From: Luca Weiss @ 2024-04-06 15:27 UTC (permalink / raw)
To: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Luca Weiss
Enable the vibrator connected to PM8941 found on the Sony shinano
platform.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
.../arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
index 3a0c0035de09..e129bb1bd6ec 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
@@ -202,6 +202,10 @@ led@7 {
};
};
+&pm8941_vib {
+ status = "okay";
+};
+
&remoteproc_adsp {
cx-supply = <&pm8841_s2>;
status = "okay";
---
base-commit: 956abeb75f90eac3d5ba1f4cff7c048f7c079502
change-id: 20240406-shinano-vib-80e27e9bab2c
Best regards,
--
Luca Weiss <luca@z3ntu.xyz>
^ 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