* Re: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
From: Dan Carpenter @ 2024-04-05 8:38 UTC (permalink / raw)
To: oe-kbuild, Damien Le Moal, Manivannan Sadhasivam,
Lorenzo Pieralisi, Kishon Vijay Abraham I, Shawn Lin,
Krzysztof Wilczyński, Bjorn Helgaas, Heiko Stuebner,
linux-pci, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
devicetree
Cc: lkp, oe-kbuild-all, linux-rockchip, linux-arm-kernel,
Rick Wertenbroek, Wilfred Mallawa, Niklas Cassel
In-Reply-To: <20240330041928.1555578-3-dlemoal@kernel.org>
Hi Damien,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Damien-Le-Moal/PCI-endpoint-Introduce-pci_epc_function_is_valid/20240330-122311
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/20240330041928.1555578-3-dlemoal%40kernel.org
patch subject: [PATCH v2 02/18] PCI: endpoint: Introduce pci_epc_map_align()
config: parisc-randconfig-r071-20240405 (https://download.01.org/0day-ci/archive/20240405/202404051508.hvNRDVZq-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202404051508.hvNRDVZq-lkp@intel.com/
smatch warnings:
drivers/pci/endpoint/pci-epc-core.c:493 pci_epc_map_align() error: we previously assumed 'features' could be null (see line 487)
vim +/features +493 drivers/pci/endpoint/pci-epc-core.c
9d2f10d2ace040 Damien Le Moal 2024-03-30 458 int pci_epc_map_align(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
9d2f10d2ace040 Damien Le Moal 2024-03-30 459 u64 pci_addr, size_t size, struct pci_epc_map *map)
9d2f10d2ace040 Damien Le Moal 2024-03-30 460 {
9d2f10d2ace040 Damien Le Moal 2024-03-30 461 const struct pci_epc_features *features;
9d2f10d2ace040 Damien Le Moal 2024-03-30 462 size_t mask;
9d2f10d2ace040 Damien Le Moal 2024-03-30 463 int ret;
9d2f10d2ace040 Damien Le Moal 2024-03-30 464
9d2f10d2ace040 Damien Le Moal 2024-03-30 465 if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
9d2f10d2ace040 Damien Le Moal 2024-03-30 466 return -EINVAL;
9d2f10d2ace040 Damien Le Moal 2024-03-30 467
9d2f10d2ace040 Damien Le Moal 2024-03-30 468 if (!size || !map)
9d2f10d2ace040 Damien Le Moal 2024-03-30 469 return -EINVAL;
9d2f10d2ace040 Damien Le Moal 2024-03-30 470
9d2f10d2ace040 Damien Le Moal 2024-03-30 471 memset(map, 0, sizeof(*map));
9d2f10d2ace040 Damien Le Moal 2024-03-30 472 map->pci_addr = pci_addr;
9d2f10d2ace040 Damien Le Moal 2024-03-30 473 map->pci_size = size;
9d2f10d2ace040 Damien Le Moal 2024-03-30 474
9d2f10d2ace040 Damien Le Moal 2024-03-30 475 if (epc->ops->map_align) {
9d2f10d2ace040 Damien Le Moal 2024-03-30 476 mutex_lock(&epc->lock);
9d2f10d2ace040 Damien Le Moal 2024-03-30 477 ret = epc->ops->map_align(epc, func_no, vfunc_no, map);
9d2f10d2ace040 Damien Le Moal 2024-03-30 478 mutex_unlock(&epc->lock);
9d2f10d2ace040 Damien Le Moal 2024-03-30 479 return ret;
9d2f10d2ace040 Damien Le Moal 2024-03-30 480 }
9d2f10d2ace040 Damien Le Moal 2024-03-30 481
9d2f10d2ace040 Damien Le Moal 2024-03-30 482 /*
9d2f10d2ace040 Damien Le Moal 2024-03-30 483 * Assume a fixed alignment constraint as specified by the controller
9d2f10d2ace040 Damien Le Moal 2024-03-30 484 * features.
9d2f10d2ace040 Damien Le Moal 2024-03-30 485 */
9d2f10d2ace040 Damien Le Moal 2024-03-30 486 features = pci_epc_get_features(epc, func_no, vfunc_no);
9d2f10d2ace040 Damien Le Moal 2024-03-30 @487 if (!features || !features->align) {
^^^^^^^^^
Check for NULL
9d2f10d2ace040 Damien Le Moal 2024-03-30 488 map->map_pci_addr = pci_addr;
9d2f10d2ace040 Damien Le Moal 2024-03-30 489 map->map_size = size;
9d2f10d2ace040 Damien Le Moal 2024-03-30 490 map->map_ofst = 0;
Missing return 0?
9d2f10d2ace040 Damien Le Moal 2024-03-30 491 }
9d2f10d2ace040 Damien Le Moal 2024-03-30 492
9d2f10d2ace040 Damien Le Moal 2024-03-30 @493 mask = features->align - 1;
^^^^^^^^^^
9d2f10d2ace040 Damien Le Moal 2024-03-30 494 map->map_pci_addr = map->pci_addr & ~mask;
9d2f10d2ace040 Damien Le Moal 2024-03-30 495 map->map_ofst = map->pci_addr & mask;
9d2f10d2ace040 Damien Le Moal 2024-03-30 496 map->map_size = ALIGN(map->map_ofst + map->pci_size, features->align);
9d2f10d2ace040 Damien Le Moal 2024-03-30 497
9d2f10d2ace040 Damien Le Moal 2024-03-30 498 return 0;
9d2f10d2ace040 Damien Le Moal 2024-03-30 499 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH 0/6] Add SMEM-based speedbin matching
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
Newer (SM8550+) SoCs don't seem to have a nice speedbin fuse anymore,
but instead rely on a set of combinations of "feature code" (FC) and
"product code" (PC) identifiers to match the bins. This series adds
support for that.
I suppose a qcom/for-soc immutable branch would be in order if we want
to land this in the upcoming cycle.
FWIW I preferred the fuses myself..
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
Konrad Dybcio (5):
soc: qcom: Move some socinfo defines to the header, expand them
soc: qcom: smem: Add pcode/fcode getters
drm/msm/adreno: Implement SMEM-based speed bin
drm/msm/adreno: Add speedbin data for SM8550 / A740
arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
Neil Armstrong (1):
drm/msm/adreno: Allow specifying default speedbin value
arch/arm64/boot/dts/qcom/sm8550.dtsi | 21 +++++++++-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 10 +++--
drivers/gpu/drm/msm/adreno/adreno_device.c | 16 ++++++++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 39 ++++++++++++++++--
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 13 ++++--
drivers/soc/qcom/smem.c | 66 ++++++++++++++++++++++++++++++
drivers/soc/qcom/socinfo.c | 8 ----
include/linux/soc/qcom/smem.h | 2 +
include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++
9 files changed, 191 insertions(+), 20 deletions(-)
---
base-commit: 2b3d5988ae2cb5cd945ddbc653f0a71706231fdd
change-id: 20240404-topic-smem_speedbin-8deecd0bef0e
Best regards,
--
Konrad Dybcio <konrad.dybcio@linaro.org>
^ permalink raw reply
* [PATCH 1/6] soc: qcom: Move some socinfo defines to the header, expand them
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
In preparation for parsing the chip "feature code" (FC) and "product
code" (PC) (essentially the parameters that let us conclusively
characterize the sillicon we're running on, including various speed
bins), move the socinfo version defines to the public header and
include some more FC/PC defines.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/soc/qcom/socinfo.c | 8 --------
include/linux/soc/qcom/socinfo.h | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/qcom/socinfo.c b/drivers/soc/qcom/socinfo.c
index 277c07a6603d..cf4616a468f2 100644
--- a/drivers/soc/qcom/socinfo.c
+++ b/drivers/soc/qcom/socinfo.c
@@ -21,14 +21,6 @@
#include <dt-bindings/arm/qcom,ids.h>
-/*
- * SoC version type with major number in the upper 16 bits and minor
- * number in the lower 16 bits.
- */
-#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
-#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
-#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
-
/* Helper macros to create soc_id table */
#define qcom_board_id(id) QCOM_ID_ ## id, __stringify(id)
#define qcom_board_id_named(id, name) QCOM_ID_ ## id, (name)
diff --git a/include/linux/soc/qcom/socinfo.h b/include/linux/soc/qcom/socinfo.h
index e78777bb0f4a..ba7f683bd32c 100644
--- a/include/linux/soc/qcom/socinfo.h
+++ b/include/linux/soc/qcom/socinfo.h
@@ -3,6 +3,8 @@
#ifndef __QCOM_SOCINFO_H__
#define __QCOM_SOCINFO_H__
+#include <linux/types.h>
+
/*
* SMEM item id, used to acquire handles to respective
* SMEM region.
@@ -12,6 +14,14 @@
#define SMEM_SOCINFO_BUILD_ID_LENGTH 32
#define SMEM_SOCINFO_CHIP_ID_LENGTH 32
+/*
+ * SoC version type with major number in the upper 16 bits and minor
+ * number in the lower 16 bits.
+ */
+#define SOCINFO_MAJOR(ver) (((ver) >> 16) & 0xffff)
+#define SOCINFO_MINOR(ver) ((ver) & 0xffff)
+#define SOCINFO_VERSION(maj, min) ((((maj) & 0xffff) << 16)|((min) & 0xffff))
+
/* Socinfo SMEM item structure */
struct socinfo {
__le32 fmt;
@@ -74,4 +84,30 @@ struct socinfo {
__le32 boot_core;
};
+/* Internal feature codes */
+enum feature_code {
+ /* External feature codes */
+ SOCINFO_FC_UNKNOWN = 0x0,
+ SOCINFO_FC_AA,
+ SOCINFO_FC_AB,
+ SOCINFO_FC_AC,
+ SOCINFO_FC_AD,
+ SOCINFO_FC_AE,
+ SOCINFO_FC_AF,
+ SOCINFO_FC_AG,
+ SOCINFO_FC_AH,
+ SOCINFO_FC_EXT_RESERVE,
+};
+
+/* Internal feature codes */
+/* Valid values: 0 <= n <= 0xf */
+#define SOCINFO_FC_Yn(n) (0xf1 + n)
+#define SOCINFO_FC_INT_RESERVE SOCINFO_FC_Yn(0x10)
+
+/* Product codes */
+#define SOCINFO_PC_UNKNOWN 0
+/* Valid values: 0 <= n <= 8, the rest is reserved */
+#define SOCINFO_PCn(n) (n + 1)
+#define SOCINFO_PC_RESERVE (BIT(31) - 1)
+
#endif
--
2.40.1
^ permalink raw reply related
* [PATCH 2/6] soc: qcom: smem: Add pcode/fcode getters
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
Introduce getters for SoC product and feature codes and export them.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/soc/qcom/smem.c | 66 +++++++++++++++++++++++++++++++++++++++++++
include/linux/soc/qcom/smem.h | 2 ++
2 files changed, 68 insertions(+)
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 7191fa0c087f..e89b4d26877a 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -795,6 +795,72 @@ int qcom_smem_get_soc_id(u32 *id)
}
EXPORT_SYMBOL_GPL(qcom_smem_get_soc_id);
+/**
+ * qcom_smem_get_feature_code() - return the feature code
+ * @id: On success, we return the feature code here.
+ *
+ * Look up the feature code identifier from SMEM and return it.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int qcom_smem_get_feature_code(u32 *code)
+{
+ struct socinfo *info;
+ u32 raw_code;
+
+ info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
+ /* This only makes sense for socinfo >= 16 */
+ if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
+ return -EINVAL;
+
+ raw_code = __le32_to_cpu(info->feature_code);
+
+ /* Ensure the value makes sense */
+ if (raw_code >= SOCINFO_FC_INT_RESERVE)
+ raw_code = SOCINFO_FC_UNKNOWN;
+
+ *code = raw_code;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_smem_get_feature_code);
+
+/**
+ * qcom_smem_get_product_code() - return the product code
+ * @id: On success, we return the product code here.
+ *
+ * Look up feature code identifier from SMEM and return it.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int qcom_smem_get_product_code(u32 *code)
+{
+ struct socinfo *info;
+ u32 raw_code;
+
+ info = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_HW_SW_BUILD_ID, NULL);
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
+ /* This only makes sense for socinfo >= 16 */
+ if (__le32_to_cpu(info->fmt) < SOCINFO_VERSION(0, 16))
+ return -EINVAL;
+
+ raw_code = __le32_to_cpu(info->pcode);
+
+ /* Ensure the value makes sense */
+ if (raw_code >= SOCINFO_FC_INT_RESERVE)
+ raw_code = SOCINFO_FC_UNKNOWN;
+
+ *code = raw_code;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_smem_get_product_code);
+
static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
{
struct smem_header *header;
diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
index a36a3b9d4929..aef8c9fc6c08 100644
--- a/include/linux/soc/qcom/smem.h
+++ b/include/linux/soc/qcom/smem.h
@@ -13,5 +13,7 @@ int qcom_smem_get_free_space(unsigned host);
phys_addr_t qcom_smem_virt_to_phys(void *p);
int qcom_smem_get_soc_id(u32 *id);
+int qcom_smem_get_feature_code(u32 *code);
+int qcom_smem_get_product_code(u32 *code);
#endif
--
2.40.1
^ permalink raw reply related
* [PATCH 3/6] drm/msm/adreno: Allow specifying default speedbin value
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
From: Neil Armstrong <neil.armstrong@linaro.org>
Usually, speedbin 0 is the "super SKU", a.k.a the one which can clock
the highest. Falling back to it when things go wrong is largely
suboptimal, as more often than not, the top frequencies are not
supposed to work on other bins.
Let the developer specify the intended "lowest common denominator" bin
in struct adreno_info. If not specified, partial struct initialization
will ensure it's set to zero, retaining previous behavior.
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
[Konrad: clean up, add commit message]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +-
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 0674aca0f8a3..4cbdfabbcee5 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2915,7 +2915,7 @@ static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *i
DRM_DEV_ERROR(dev,
"missing support for speed-bin: %u. Some OPPs may not be supported by hardware\n",
speedbin);
- supp_hw = BIT(0); /* Default */
+ supp_hw = BIT(info->default_speedbin); /* Default */
}
ret = devm_pm_opp_set_supported_hw(dev, &supp_hw, 1);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 77526892eb8c..460b399be37b 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -110,6 +110,7 @@ struct adreno_info {
* {SHRT_MAX, 0} sentinal.
*/
struct adreno_speedbin *speedbins;
+ unsigned int default_speedbin;
};
#define ADRENO_CHIP_IDS(tbl...) (uint32_t[]) { tbl, 0 }
--
2.40.1
^ permalink raw reply related
* [PATCH 4/6] drm/msm/adreno: Implement SMEM-based speed bin
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
On recent (SM8550+) Snapdragon platforms, the GPU speed bin data is
abstracted through SMEM, instead of being directly available in a fuse.
Add support for SMEM-based speed binning, which includes getting
"feature code" and "product code" from said source and parsing them
to form something that lets us match OPPs against.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 +++---
drivers/gpu/drm/msm/adreno/adreno_device.c | 2 ++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 39 +++++++++++++++++++++++++++---
drivers/gpu/drm/msm/adreno/adreno_gpu.h | 12 ++++++---
4 files changed, 51 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 4cbdfabbcee5..6776fd80f7a6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -2890,13 +2890,15 @@ static u32 fuse_to_supp_hw(const struct adreno_info *info, u32 fuse)
return UINT_MAX;
}
-static int a6xx_set_supported_hw(struct device *dev, const struct adreno_info *info)
+static int a6xx_set_supported_hw(struct adreno_gpu *adreno_gpu,
+ struct device *dev,
+ const struct adreno_info *info)
{
u32 supp_hw;
u32 speedbin;
int ret;
- ret = adreno_read_speedbin(dev, &speedbin);
+ ret = adreno_read_speedbin(adreno_gpu, dev, &speedbin);
/*
* -ENOENT means that the platform doesn't support speedbin which is
* fine
@@ -3056,7 +3058,7 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
a6xx_llc_slices_init(pdev, a6xx_gpu, is_a7xx);
- ret = a6xx_set_supported_hw(&pdev->dev, config->info);
+ ret = a6xx_set_supported_hw(adreno_gpu, &pdev->dev, config->info);
if (ret) {
a6xx_destroy(&(a6xx_gpu->base.base));
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index c3703a51287b..901ef767e491 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -6,6 +6,8 @@
* Copyright (c) 2014,2017 The Linux Foundation. All rights reserved.
*/
+#include <linux/soc/qcom/socinfo.h>
+
#include "adreno_gpu.h"
bool hang_debug = false;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 074fb498706f..0e4ff532ac3c 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -21,6 +21,9 @@
#include "msm_gem.h"
#include "msm_mmu.h"
+#include <linux/soc/qcom/smem.h>
+#include <linux/soc/qcom/socinfo.h>
+
static u64 address_space_size = 0;
MODULE_PARM_DESC(address_space_size, "Override for size of processes private GPU address space");
module_param(address_space_size, ullong, 0600);
@@ -1057,9 +1060,37 @@ void adreno_gpu_ocmem_cleanup(struct adreno_ocmem *adreno_ocmem)
adreno_ocmem->hdl);
}
-int adreno_read_speedbin(struct device *dev, u32 *speedbin)
+int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
+ struct device *dev, u32 *speedbin)
{
- return nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
+ u32 fcode, pcode;
+ int ret;
+
+ /* Try reading the speedbin via a nvmem cell first */
+ ret = nvmem_cell_read_variable_le_u32(dev, "speed_bin", speedbin);
+ if (!ret && ret != -EINVAL)
+ return ret;
+
+ ret = qcom_smem_get_feature_code(&fcode);
+ if (ret) {
+ dev_err(dev, "Couldn't get feature code from SMEM!\n");
+ return ret;
+ }
+
+ ret = qcom_smem_get_product_code(&pcode);
+ if (ret) {
+ dev_err(dev, "Couldn't get product code from SMEM!\n");
+ return ret;
+ }
+
+ /* Don't consider fcode for external feature codes */
+ if (fcode <= SOCINFO_FC_EXT_RESERVE)
+ fcode = SOCINFO_FC_UNKNOWN;
+
+ *speedbin = FIELD_PREP(ADRENO_SKU_ID_PCODE, pcode) |
+ FIELD_PREP(ADRENO_SKU_ID_FCODE, fcode);
+
+ return ret;
}
int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
@@ -1098,9 +1129,9 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
devm_pm_opp_set_clkname(dev, "core");
}
- if (adreno_read_speedbin(dev, &speedbin) || !speedbin)
+ if (adreno_read_speedbin(adreno_gpu, dev, &speedbin) || !speedbin)
speedbin = 0xffff;
- adreno_gpu->speedbin = (uint16_t) (0xffff & speedbin);
+ adreno_gpu->speedbin = speedbin;
gpu_name = devm_kasprintf(dev, GFP_KERNEL, "%"ADRENO_CHIPID_FMT,
ADRENO_CHIPID_ARGS(config->chip_id));
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index 460b399be37b..1770a9e20484 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -81,7 +81,12 @@ extern const struct adreno_reglist a612_hwcg[], a615_hwcg[], a630_hwcg[], a640_h
extern const struct adreno_reglist a660_hwcg[], a690_hwcg[], a702_hwcg[], a730_hwcg[], a740_hwcg[];
struct adreno_speedbin {
- uint16_t fuse;
+ /* <= 16-bit for NVMEM fuses, 32b for SOCID values */
+ uint32_t fuse;
+#define ADRENO_SKU_ID_PCODE GENMASK(31, 16)
+#define ADRENO_SKU_ID_FCODE GENMASK(15, 0)
+#define ADRENO_SKU_ID(pcode, fcode) (pcode << 16 | fcode)
+
uint16_t speedbin;
};
@@ -137,7 +142,7 @@ struct adreno_gpu {
struct msm_gpu base;
const struct adreno_info *info;
uint32_t chip_id;
- uint16_t speedbin;
+ uint32_t speedbin;
const struct adreno_gpu_funcs *funcs;
/* interesting register offsets to dump: */
@@ -520,7 +525,8 @@ int adreno_fault_handler(struct msm_gpu *gpu, unsigned long iova, int flags,
struct adreno_smmu_fault_info *info, const char *block,
u32 scratch[4]);
-int adreno_read_speedbin(struct device *dev, u32 *speedbin);
+int adreno_read_speedbin(struct adreno_gpu *adreno_gpu,
+ struct device *dev, u32 *speedbin);
/*
* For a5xx and a6xx targets load the zap shader that is used to pull the GPU
--
2.40.1
^ permalink raw reply related
* [PATCH 5/6] drm/msm/adreno: Add speedbin data for SM8550 / A740
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
Add speebin data for A740, as found on SM8550 and derivative SoCs.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/gpu/drm/msm/adreno/adreno_device.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 901ef767e491..c976a485aef2 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -570,6 +570,20 @@ static const struct adreno_info gpulist[] = {
.zapfw = "a740_zap.mdt",
.hwcg = a740_hwcg,
.address_space_size = SZ_16G,
+ .speedbins = ADRENO_SPEEDBINS(
+ { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AC), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PC_UNKNOWN, SOCINFO_FC_AF), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(1), SOCINFO_FC_UNKNOWN), 1 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x0)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(2), SOCINFO_FC_Yn(0x2)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x0)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(4), SOCINFO_FC_Yn(0x2)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x0)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0x1)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xd)), 0 },
+ { ADRENO_SKU_ID(SOCINFO_PCn(6), SOCINFO_FC_Yn(0xe)), 0 },
+ ),
+ .default_speedbin = 1,
}, {
.chip_ids = ADRENO_CHIP_IDS(0x43051401), /* "C520v2" */
.family = ADRENO_7XX_GEN3,
--
2.40.1
^ permalink raw reply related
* [PATCH 6/6] arm64: dts: qcom: sm8550: Wire up GPU speed bin & more OPPs
From: Konrad Dybcio @ 2024-04-05 8:41 UTC (permalink / raw)
To: Bjorn Andersson, Rob Clark, Abhinav Kumar, Dmitry Baryshkov,
Sean Paul, Marijn Suijten, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, linux-kernel, dri-devel, freedreno, devicetree,
Neil Armstrong, Konrad Dybcio
In-Reply-To: <20240405-topic-smem_speedbin-v1-0-ce2b864251b1@linaro.org>
Add the speedbin masks to ensure only the desired OPPs are available on
chips of a given bin.
Using this, add the binned 719 MHz OPP and the non-binned 124.8 MHz.
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/sm8550.dtsi | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 5cae8d773cec..2f6842f6a5b7 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -2087,48 +2087,67 @@ zap-shader {
memory-region = <&gpu_micro_code_mem>;
};
- /* Speedbin needs more work on A740+, keep only lower freqs */
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
+ opp-719000000 {
+ opp-hz = /bits/ 64 <719000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_SVS_L2>;
+ opp-supported-hw = <0x1>;
+ };
+
opp-680000000 {
opp-hz = /bits/ 64 <680000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+ opp-supported-hw = <0x3>;
};
opp-615000000 {
opp-hz = /bits/ 64 <615000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L0>;
+ opp-supported-hw = <0x3>;
};
opp-550000000 {
opp-hz = /bits/ 64 <550000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+ opp-supported-hw = <0x3>;
};
opp-475000000 {
opp-hz = /bits/ 64 <475000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_L1>;
+ opp-supported-hw = <0x3>;
};
opp-401000000 {
opp-hz = /bits/ 64 <401000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ opp-supported-hw = <0x3>;
};
opp-348000000 {
opp-hz = /bits/ 64 <348000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D0>;
+ opp-supported-hw = <0x3>;
};
opp-295000000 {
opp-hz = /bits/ 64 <295000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
+ opp-supported-hw = <0x3>;
};
opp-220000000 {
opp-hz = /bits/ 64 <220000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D2>;
+ opp-supported-hw = <0x3>;
+ };
+
+ opp-124800000 {
+ opp-hz = /bits/ 64 <124800000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D2>;
+ opp-supported-hw = <0x3>;
};
};
};
--
2.40.1
^ permalink raw reply related
* [PATCH 2/3] arm64: dts: ti: k3-am62a-main: Fix the reg-range for dma-controller
From: Jayesh Choudhary @ 2024-04-05 8:52 UTC (permalink / raw)
To: nm, vigneshr, bb, devicetree, j-choudhary
Cc: kristo, robh, krzk+dt, conor+dt, linux-kernel, linux-arm-kernel
In-Reply-To: <20240405085208.32227-1-j-choudhary@ti.com>
The TX Channel Realtime Registers region 'tchanrt' is 128KB and Ring
Realtime Registers region 'ringrt' is 2MB as shown in memory map in
the TRM[0]. So fix ranges for those reg-regions.
[0]: <https://www.ti.com/lit/pdf/spruj16>
Fixes: 3dad70def7ff ("arm64: dts: ti: k3-am62a-main: Add more peripheral nodes")
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index aa1e057082f0..5a4cb0536c6f 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -120,8 +120,8 @@ main_pktdma: dma-controller@485c0000 {
compatible = "ti,am64-dmss-pktdma";
reg = <0x00 0x485c0000 0x00 0x100>,
<0x00 0x4a800000 0x00 0x20000>,
- <0x00 0x4aa00000 0x00 0x40000>,
- <0x00 0x4b800000 0x00 0x400000>,
+ <0x00 0x4aa00000 0x00 0x20000>,
+ <0x00 0x4b800000 0x00 0x200000>,
<0x00 0x485e0000 0x00 0x10000>,
<0x00 0x484a0000 0x00 0x2000>,
<0x00 0x484c0000 0x00 0x2000>,
--
2.25.1
^ permalink raw reply related
* [PATCH 1/3] arm64: dts: ti: k3-am62-main: Fix the reg-range for dma-controller
From: Jayesh Choudhary @ 2024-04-05 8:52 UTC (permalink / raw)
To: nm, vigneshr, bb, devicetree, j-choudhary
Cc: kristo, robh, krzk+dt, conor+dt, linux-kernel, linux-arm-kernel
In-Reply-To: <20240405085208.32227-1-j-choudhary@ti.com>
The TX Channel Realtime Registers region 'tchanrt' is 128KB and Ring
Realtime Registers region 'ringrt' is 2MB as shown in memory map in
the TRM[0]. So fix ranges for those reg-regions.
[0]: <https://www.ti.com/lit/pdf/spruiv7>
Fixes: c37c58fdeb8a ("arm64: dts: ti: k3-am62: Add more peripheral nodes")
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index e9cffca073ef..e10cc9fc0b10 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -141,8 +141,8 @@ main_pktdma: dma-controller@485c0000 {
compatible = "ti,am64-dmss-pktdma";
reg = <0x00 0x485c0000 0x00 0x100>,
<0x00 0x4a800000 0x00 0x20000>,
- <0x00 0x4aa00000 0x00 0x40000>,
- <0x00 0x4b800000 0x00 0x400000>,
+ <0x00 0x4aa00000 0x00 0x20000>,
+ <0x00 0x4b800000 0x00 0x200000>,
<0x00 0x485e0000 0x00 0x10000>,
<0x00 0x484a0000 0x00 0x2000>,
<0x00 0x484c0000 0x00 0x2000>,
--
2.25.1
^ permalink raw reply related
* [PATCH 0/3] Fix reg ranges for dma-controller node
From: Jayesh Choudhary @ 2024-04-05 8:52 UTC (permalink / raw)
To: nm, vigneshr, bb, devicetree, j-choudhary
Cc: kristo, robh, krzk+dt, conor+dt, linux-kernel, linux-arm-kernel
The dma-controller node 'main_pktdma' has few memory regions with
wrong sizes.
DMASS0_PKTDMA_RINGRT is marked as 4MB region when it is actually a 2MB
region. Similarly, DMASS0_PKTDMA_TCHANRT is marked as 256KB region but
the actual size is 128KB as shown in TRM in the section for Main Memory
Map (Table 2-1)
Fix these region across AM62, AM62A and AM62P (which is also used in
J722S)
TRM:
AM625: <https://www.ti.com/lit/pdf/spruiv7>
AM62A7: <https://www.ti.com/lit/pdf/spruj16>
AM62P: <https://www.ti.com/lit/pdf/spruj83>
J722S: <https://www.ti.com/lit/zip/sprujb3>
Jayesh Choudhary (3):
arm64: dts: ti: k3-am62-main: Fix the reg-range for dma-controller
arm64: dts: ti: k3-am62a-main: Fix the reg-range for dma-controller
arm64: dts: ti: k3-am62p-main: Fix the reg-range for dma-controller
arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 4 ++--
arch/arm64/boot/dts/ti/k3-am62a-main.dtsi | 4 ++--
arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH 3/3] arm64: dts: ti: k3-am62p-main: Fix the reg-range for dma-controller
From: Jayesh Choudhary @ 2024-04-05 8:52 UTC (permalink / raw)
To: nm, vigneshr, bb, devicetree, j-choudhary
Cc: kristo, robh, krzk+dt, conor+dt, linux-kernel, linux-arm-kernel
In-Reply-To: <20240405085208.32227-1-j-choudhary@ti.com>
The TX Channel Realtime Registers region 'tchanrt' is 128KB and Ring
Realtime Registers region 'ringrt' is 2MB as shown in memory map in
the TRM[0]. So fix ranges for those reg-regions.
[0]: <https://www.ti.com/lit/pdf/spruj83>
Fixes: b5080c7c1f7e ("arm64: dts: ti: k3-am62p: Add nodes for more IPs")
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-am62p-main.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
index 7337a9e13535..514c201bd5c9 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
@@ -123,8 +123,8 @@ main_pktdma: dma-controller@485c0000 {
compatible = "ti,am64-dmss-pktdma";
reg = <0x00 0x485c0000 0x00 0x100>,
<0x00 0x4a800000 0x00 0x20000>,
- <0x00 0x4aa00000 0x00 0x40000>,
- <0x00 0x4b800000 0x00 0x400000>,
+ <0x00 0x4aa00000 0x00 0x20000>,
+ <0x00 0x4b800000 0x00 0x200000>,
<0x00 0x485e0000 0x00 0x10000>,
<0x00 0x484a0000 0x00 0x2000>,
<0x00 0x484c0000 0x00 0x2000>,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v12 2/7] clk: meson: add vclk driver
From: Jerome Brunet @ 2024-04-05 7:00 UTC (permalink / raw)
To: neil.armstrong
Cc: Jerome Brunet, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Martin Blumenstingl, Kevin Hilman, Michael Turquette,
Stephen Boyd, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Jagan Teki, Nicolas Belin,
devicetree, linux-kernel, linux-amlogic, linux-clk,
linux-arm-kernel, dri-devel
In-Reply-To: <2cf79f07-0ae1-4267-ac08-fe40366d87d4@linaro.org>
On Thu 04 Apr 2024 at 18:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> On 04/04/2024 10:13, Jerome Brunet wrote:
>> On Wed 03 Apr 2024 at 09:46, Neil Armstrong <neil.armstrong@linaro.org>
>> wrote:
>>
>>> The VCLK and VCLK_DIV clocks have supplementary bits.
>>>
>>> The VCLK gate has a "SOFT RESET" bit to toggle after the whole
>>> VCLK sub-tree rate has been set, this is implemented in
>>> the gate enable callback.
>>>
>>> The VCLK_DIV clocks as enable and reset bits used to disable
>>> and reset the divider, associated with CLK_SET_RATE_GATE it ensures
>>> the rate is set while the divider is disabled and in reset mode.
>>>
>>> The VCLK_DIV enable bit isn't implemented as a gate since it's part
>>> of the divider logic and vendor does this exact sequence to ensure
>>> the divider is correctly set.
>> The checkpatch warning is still there. Is it a choice or a mistake ?
>> Documentation says "GPL v2" exists for historic reason which seems to
>> hint "GPL" is preferred, and I suppose this is why checkpatch warns for
>> it.
>
> Well I didn't see this warning, this is what I fixed:
>
> $ scripts/checkpatch.pl --strict drivers/clk/meson/vclk.c
> CHECK: Alignment should match open parenthesis
> #63: FILE: drivers/clk/meson/vclk.c:63:
> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
> + unsigned long prate)
>
> CHECK: Alignment should match open parenthesis
> #73: FILE: drivers/clk/meson/vclk.c:73:
> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
>
> CHECK: Alignment should match open parenthesis
> #83: FILE: drivers/clk/meson/vclk.c:83:
> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
>
I would not ask a respin solely for this. It's nice to fix it but I was
mostly after the warning TBH.
> <snip>
>
> It seems that checking a commit triggers an extra check....
>
> $ scripts/checkpatch.pl --strict -G 1bac9f6aa3c3
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #58:
> new file mode 100644
>
> <snip>
>
> WARNING: Prefer "GPL" over "GPL v2" - see commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL v2" bogosity")
> #203: FILE: drivers/clk/meson/vclk.c:141:
> +MODULE_LICENSE("GPL v2");
Hum, I'm running checkpatch against the mail itself, not the commit. I
still get the warning
>
> <snip>
>
> Neil
>
>>
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>> drivers/clk/meson/Kconfig | 4 ++
>>> drivers/clk/meson/Makefile | 1 +
>>> drivers/clk/meson/vclk.c | 141 +++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/clk/meson/vclk.h | 51 ++++++++++++++++
>>> 4 files changed, 197 insertions(+)
>>>
>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>> index 29ffd14d267b..8a9823789fa3 100644
>>> --- a/drivers/clk/meson/Kconfig
>>> +++ b/drivers/clk/meson/Kconfig
>>> @@ -30,6 +30,10 @@ config COMMON_CLK_MESON_VID_PLL_DIV
>>> tristate
>>> select COMMON_CLK_MESON_REGMAP
>>> +config COMMON_CLK_MESON_VCLK
>>> + tristate
>>> + select COMMON_CLK_MESON_REGMAP
>>> +
>>> config COMMON_CLK_MESON_CLKC_UTILS
>>> tristate
>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>> index 9ee4b954c896..9ba43fe7a07a 100644
>>> --- a/drivers/clk/meson/Makefile
>>> +++ b/drivers/clk/meson/Makefile
>>> @@ -12,6 +12,7 @@ obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
>>> obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
>>> +obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
>>> # Amlogic Clock controllers
>>> diff --git a/drivers/clk/meson/vclk.c b/drivers/clk/meson/vclk.c
>>> new file mode 100644
>>> index 000000000000..45dc216941ea
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/vclk.c
>>> @@ -0,0 +1,141 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright (c) 2024 Neil Armstrong <neil.armstrong@linaro.org>
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include "vclk.h"
>>> +
>>> +/* The VCLK gate has a supplementary reset bit to pulse after ungating */
>>> +
>>> +static inline struct meson_vclk_gate_data *
>>> +clk_get_meson_vclk_gate_data(struct clk_regmap *clk)
>>> +{
>>> + return (struct meson_vclk_gate_data *)clk->data;
>>> +}
>>> +
>>> +static int meson_vclk_gate_enable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>> +
>>> + /* Do a reset pulse */
>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_vclk_gate_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>> +}
>>> +
>>> +static int meson_vclk_gate_is_enabled(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_gate_data *vclk = clk_get_meson_vclk_gate_data(clk);
>>> +
>>> + return meson_parm_read(clk->map, &vclk->enable);
>>> +}
>>> +
>>> +const struct clk_ops meson_vclk_gate_ops = {
>>> + .enable = meson_vclk_gate_enable,
>>> + .disable = meson_vclk_gate_disable,
>>> + .is_enabled = meson_vclk_gate_is_enabled,
>>> +};
>>> +EXPORT_SYMBOL_GPL(meson_vclk_gate_ops);
>>> +
>>> +/* The VCLK Divider has supplementary reset & enable bits */
>>> +
>>> +static inline struct meson_vclk_div_data *
>>> +clk_get_meson_vclk_div_data(struct clk_regmap *clk)
>>> +{
>>> + return (struct meson_vclk_div_data *)clk->data;
>>> +}
>>> +
>>> +static unsigned long meson_vclk_div_recalc_rate(struct clk_hw *hw,
>>> + unsigned long prate)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return divider_recalc_rate(hw, prate, meson_parm_read(clk->map, &vclk->div),
>>> + vclk->table, vclk->flags, vclk->div.width);
>>> +}
>>> +
>>> +static int meson_vclk_div_determine_rate(struct clk_hw *hw,
>>> + struct clk_rate_request *req)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return divider_determine_rate(hw, req, vclk->table, vclk->div.width,
>>> + vclk->flags);
>>> +}
>>> +
>>> +static int meson_vclk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> + int ret;
>>> +
>>> + ret = divider_get_val(rate, parent_rate, vclk->table, vclk->div.width,
>>> + vclk->flags);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + meson_parm_write(clk->map, &vclk->div, ret);
>>> +
>>> + return 0;
>>> +};
>>> +
>>> +static int meson_vclk_div_enable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + /* Unreset the divider when ungating */
>>> + meson_parm_write(clk->map, &vclk->reset, 0);
>>> + meson_parm_write(clk->map, &vclk->enable, 1);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void meson_vclk_div_disable(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + /* Reset the divider when gating */
>>> + meson_parm_write(clk->map, &vclk->enable, 0);
>>> + meson_parm_write(clk->map, &vclk->reset, 1);
>>> +}
>>> +
>>> +static int meson_vclk_div_is_enabled(struct clk_hw *hw)
>>> +{
>>> + struct clk_regmap *clk = to_clk_regmap(hw);
>>> + struct meson_vclk_div_data *vclk = clk_get_meson_vclk_div_data(clk);
>>> +
>>> + return meson_parm_read(clk->map, &vclk->enable);
>>> +}
>>> +
>>> +const struct clk_ops meson_vclk_div_ops = {
>>> + .recalc_rate = meson_vclk_div_recalc_rate,
>>> + .determine_rate = meson_vclk_div_determine_rate,
>>> + .set_rate = meson_vclk_div_set_rate,
>>> + .enable = meson_vclk_div_enable,
>>> + .disable = meson_vclk_div_disable,
>>> + .is_enabled = meson_vclk_div_is_enabled,
>>> +};
>>> +EXPORT_SYMBOL_GPL(meson_vclk_div_ops);
>>> +
>>> +MODULE_DESCRIPTION("Amlogic vclk clock driver");
>>> +MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
>>> new file mode 100644
>>> index 000000000000..20b0b181db09
>>> --- /dev/null
>>> +++ b/drivers/clk/meson/vclk.h
>>> @@ -0,0 +1,51 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * Copyright (c) 2024 Neil Armstrong <neil.armstrong@linaro.org>
>>> + */
>>> +
>>> +#ifndef __VCLK_H
>>> +#define __VCLK_H
>>> +
>>> +#include "clk-regmap.h"
>>> +#include "parm.h"
>>> +
>>> +/**
>>> + * struct meson_vclk_gate_data - vclk_gate regmap backed specific data
>>> + *
>>> + * @enable: vclk enable field
>>> + * @reset: vclk reset field
>>> + * @flags: hardware-specific flags
>>> + *
>>> + * Flags:
>>> + * Same as clk_gate except CLK_GATE_HIWORD_MASK which is ignored
>>> + */
>>> +struct meson_vclk_gate_data {
>>> + struct parm enable;
>>> + struct parm reset;
>>> + u8 flags;
>>> +};
>>> +
>>> +extern const struct clk_ops meson_vclk_gate_ops;
>>> +
>>> +/**
>>> + * struct meson_vclk_div_data - vclk_div regmap back specific data
>>> + *
>>> + * @div: divider field
>>> + * @enable: vclk divider enable field
>>> + * @reset: vclk divider reset field
>>> + * @table: array of value/divider pairs, last entry should have div = 0
>>> + *
>>> + * Flags:
>>> + * Same as clk_divider except CLK_DIVIDER_HIWORD_MASK which is ignored
>>> + */
>>> +struct meson_vclk_div_data {
>>> + struct parm div;
>>> + struct parm enable;
>>> + struct parm reset;
>>> + const struct clk_div_table *table;
>>> + u8 flags;
>>> +};
>>> +
>>> +extern const struct clk_ops meson_vclk_div_ops;
>>> +
>>> +#endif /* __VCLK_H */
>>
--
Jerome
^ permalink raw reply
* [PATCH v1 0/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-05 9:01 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
This patch series updates the BT firmware file names in BTNXPUART
driver, and adds a new optional firmware-name device tree property to
override the firmware file names hardcoded in the driver. This will
allow user to continue using the older firmware files.
This change is necessary as newer firmware releases will have
standardized naming convention aligned across all newer and legacy
chipsets.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Neeraj Sanjay Kale (2):
dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
Bluetooth: btnxpuart: Update firmware names
.../bindings/net/bluetooth/nxp,88w8987-bt.yaml | 8 ++++++++
drivers/bluetooth/btnxpuart.c | 12 +++++++++---
2 files changed, 17 insertions(+), 3 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v1 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Neeraj Sanjay Kale @ 2024-04-05 9:01 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240405090118.582310-1-neeraj.sanjaykale@nxp.com>
This adds a new optional device tree property called firware-name.
If this property is specified, the driver overrides the firmware
name hardcoded in the driver source code.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
.../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
index f01a3988538c..25c258212bcd 100644
--- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
@@ -31,6 +31,13 @@ properties:
This property depends on the module vendor's
configuration.
+ firmware-name:
+ $ref: /schemas/types.yaml#/definitions/string
+ description:
+ Specify firmware file name, prepended with nxp/.
+ This property overrides the firmware names hardcoded
+ in the driver source code.
+
required:
- compatible
@@ -42,5 +49,6 @@ examples:
bluetooth {
compatible = "nxp,88w8987-bt";
fw-init-baudrate = <3000000>;
+ firmware-name = "nxp/uartuart8987_bt_v0.bin";
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH v1 2/2] Bluetooth: btnxpuart: Update firmware names
From: Neeraj Sanjay Kale @ 2024-04-05 9:01 UTC (permalink / raw)
To: marcel, luiz.dentz, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, neeraj.sanjaykale, sherry.sun,
ziniu.wang_1, haibo.chen, LnxRevLi
In-Reply-To: <20240405090118.582310-1-neeraj.sanjaykale@nxp.com>
This updates the firmware names of 3 chipsets: w8987, w8997, w9098.
These changes are been done to standardize chip specific firmware
file names.
To allow user to use older firmware file names, a new device tree
property has been introduced called firmware-name, which will override
the hardcoded firmware names in the driver.
Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
drivers/bluetooth/btnxpuart.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0b93c2ff29e4..45f2ae2b542b 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -33,9 +33,9 @@
/* NXP HW err codes */
#define BTNXPUART_IR_HW_ERR 0xb0
-#define FIRMWARE_W8987 "nxp/uartuart8987_bt.bin"
-#define FIRMWARE_W8997 "nxp/uartuart8997_bt_v4.bin"
-#define FIRMWARE_W9098 "nxp/uartuart9098_bt_v1.bin"
+#define FIRMWARE_W8987 "nxp/uart8987_bt_v0.bin"
+#define FIRMWARE_W8997 "nxp/uart8997_bt_v4.bin"
+#define FIRMWARE_W9098 "nxp/uart9098_bt_v1.bin"
#define FIRMWARE_IW416 "nxp/uartiw416_bt_v0.bin"
#define FIRMWARE_IW612 "nxp/uartspi_n61x_v1.bin.se"
#define FIRMWARE_IW624 "nxp/uartiw624_bt.bin"
@@ -685,12 +685,18 @@ static bool process_boot_signature(struct btnxpuart_dev *nxpdev)
static int nxp_request_firmware(struct hci_dev *hdev, const char *fw_name)
{
struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
+ const char *fw_name_dt;
int err = 0;
if (!fw_name)
return -ENOENT;
if (!strlen(nxpdev->fw_name)) {
+ if (strcmp(fw_name, FIRMWARE_HELPER) &&
+ !device_property_read_string(&nxpdev->serdev->dev,
+ "firmware-name",
+ &fw_name_dt))
+ fw_name = fw_name_dt;
snprintf(nxpdev->fw_name, MAX_FW_FILE_NAME_LEN, "%s", fw_name);
bt_dev_dbg(hdev, "Request Firmware: %s", nxpdev->fw_name);
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH 0/6] Support ROHM BD96801 scalable PMIC
From: Matti Vaittinen @ 2024-04-05 9:19 UTC (permalink / raw)
To: Mark Brown
Cc: Matti Vaittinen, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Wim Van Sebroeck, Guenter Roeck,
devicetree, linux-kernel, linux-watchdog
In-Reply-To: <b6279be8-cf7d-4608-b556-3c01587f0d43@gmail.com>
On 4/4/24 16:15, Matti Vaittinen wrote:
> Hi Mark,
>
> On 4/4/24 15:09, Mark Brown wrote:
>> On Thu, Apr 04, 2024 at 10:26:34AM +0300, Matti Vaittinen wrote:
>>
>>> 1. Should we be able to have more than 1 IRQ domain / device?
>>> 2. Should regmap_irq support having more than 1 HWIRQ
>>
>> I would expect each parent interrupt to show up as a separate remap_irq.
>>
>>> then it seems that reading the IRQ information from the /proc/interrupts
>>> works as expected. Here I am making a wild guess that the name of the
>>> domain
>>> is used as a key for some data-lookups, and having two domains with a
>>> same
>>> name will either overwrite something or cause wrong domain data to be
>>> fetched. (This is just guessing for now).
This was wrong guessing.
>> So if we arrange to supply a name when we register multiple domains
>> things should work fine?
After my latest findings, yes, I think so. How to do this correctly is
beyond me though. The __irq_domain_create() seems to me that the name is
meant to be the dt-node name when the controller is backed by a real
dt-node. Naming of the irq_domain_alloc_named_fwnode() sounds to me like
it is only intended to be used when there is no real fwnode. All
suggestions appreciated. Using the:
irq_domain_update_bus_token(intb_domain, DOMAIN_BUS_WIRED);
feels like a dirty hack, and won't scale if there is more HWIRQs.
> Thanks for taking the time to look at my questions :)
> I have been debugging this thing whole day today, without getting too
> far :) It seems there is something beyond the name collision though.
>
> After I tried adding '-1' to the end of the other domain name to avoid
> the debugfs name collision I managed to do couple of successful runs -
> after which I reported here that problem seems to be just the naming.
> Soon after sending that mail I hit the oops again even though the naming
> was fixed.
>
> Further debugging shows that the desc->action->name for the last 28
> 'errb' IRQs get corrupted. This might point more to the IRQ requester
> side - so I need to further study the BD96801 driver side as well as the
> regulator_irq_helper. I'm having the creeping feeling that at the end of
> the day I need to find the guilty one from the mirror :)
I was not wrong on this one. The regulator_irq_helper() duplicates
memory for the data given in const struct regulator_irq_desc *d - but
it does not duplicate the irq name pointed from the given
regulator_irq_desc. Nor does the request_threaded_irq(). I passed some
of the IRQ names from the stack in the BD96801 driver ... a bug I
should've caught earlier.
Well, good thing is that now I can fix the regulator_irq_helper() to do:
--- a/drivers/regulator/irq_helpers.c
+++ b/drivers/regulator/irq_helpers.c
@@ -352,6 +352,9 @@ void *regulator_irq_helper(struct device *dev,
h->irq = irq;
h->desc = *d;
+ h->desc.name = devm_kstrdup(dev, d->name, GFP_KERNEL);
+ if (!h->desc.name)
+ return ERR_PTR(-ENOMEM);
ret = init_rdev_state(dev, h, rdev, common_errs, per_rdev_errs,
rdev_amount);
I'll send a patch if this sounds like a correct thing to do.
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
^ permalink raw reply
* Re: [PATCH] dt-bindings: extcon: ptn5150: Document the 'port' node
From: Krzysztof Kozlowski @ 2024-04-05 9:24 UTC (permalink / raw)
To: Fabio Estevam, cw00.choi
Cc: myungjoo.ham, robh, conor+dt, devicetree, marex, Fabio Estevam
In-Reply-To: <20240404022943.528293-1-festevam@gmail.com>
On 04/04/2024 04:29, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
>
> Doument the port node to link the PTN5150 to a TypeC controller.
>
> This fixes the following dt-schema warnings:
>
> imx8mp-dhcom-pdk3.dtb: typec@3d: 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/extcon/extcon-ptn5150.yaml#
Your patch ends up in spam. Probably
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> .../devicetree/bindings/extcon/extcon-ptn5150.yaml | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/extcon/extcon-ptn5150.yaml b/Documentation/devicetree/bindings/extcon/extcon-ptn5150.yaml
> index d5cfa32ea52d..3472c69056ac 100644
> --- a/Documentation/devicetree/bindings/extcon/extcon-ptn5150.yaml
> +++ b/Documentation/devicetree/bindings/extcon/extcon-ptn5150.yaml
> @@ -36,6 +36,11 @@ properties:
> description:
> GPIO pin (output) used to control VBUS. If skipped, no such control
> takes place.
Missing blank line.
> + port:
> + $ref: /schemas/graph.yaml#/$defs/port-base
Why not "$ref: /schemas/graph.yaml#/properties/port"?
But more important, what about USB C connector?
> + description:
> + A port node to link the PTN5150 to a TypeC controller.
> + unevaluatedProperties: false
>
> required:
> - compatible
> @@ -58,5 +63,11 @@ examples:
> interrupt-parent = <&msmgpio>;
> interrupts = <78 IRQ_TYPE_LEVEL_HIGH>;
> vbus-gpios = <&msmgpio 148 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ptn5150_out_ep: endpoint {
> + remote-endpoint = <&dwc3_0_ep>;
> + };
Messed indentation. Just compare the indentation with what is around. It
seems you repeat the same mistake in your recent bindings patches.
> + };
> };
> };
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 1/2] arm64: dts: imx8mm-var-som-symphony: drop redundant status from typec
From: Krzysztof Kozlowski @ 2024-04-05 9:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
imx, linux-arm-kernel, linux-kernel
Cc: Krzysztof Kozlowski
"okay" is the default status, so drop redundant property from the typec
node.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dts
index d643381417f1..affbc67c2ef6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-var-som-symphony.dts
@@ -117,7 +117,6 @@ extcon_usbotg1: typec@3d {
interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ptn5150>;
- status = "okay";
};
};
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] arm64: dts: imx8mn-var-som-symphony: drop redundant status from typec
From: Krzysztof Kozlowski @ 2024-04-05 9:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, devicetree,
imx, linux-arm-kernel, linux-kernel
Cc: Krzysztof Kozlowski
In-Reply-To: <20240405092819.40994-1-krzysztof.kozlowski@linaro.org>
"okay" is the default status, so drop redundant property from the typec
node.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
index a6b94d1957c9..3434b189fa58 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mn-var-som-symphony.dts
@@ -126,7 +126,6 @@ extcon_usbotg1: typec@3d {
interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ptn5150>;
- status = "okay";
port {
typec1_dr_sw: endpoint {
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Kalle Valo @ 2024-04-05 9:33 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <CAMRc=MeCjNn7QdDrcQMuj32JFYoemQ6A8WOYcwKJo1YhDTfY+Q@mail.gmail.com>
Bartosz Golaszewski <brgl@bgdev.pl> writes:
> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>> >
>> > +
>> > +maintainers:
>> > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>
>> IMHO it would be better to have just driver maintainers listed here.
>>
>
> Why? What's wrong with having the author of the bindings in the Cc list?
If you want follow the ath12k development and review patches then you
can join the ath12k list. I'm not fond of having too many maintainers,
it's not really helping anything and just extra work to periodically
cleanup the silent maintainers.
I would ask the opposite question: why add you as the maintainer?
There's not even a single ath12k patch from you, nor I haven't seen you
doing any patch review or otherwise helping others related to ath12k.
Don't get me wrong, I value the work you do with this important powerseq
feature and hopefully we get it into the tree soon. But I don't see
adding you as a maintainer at this point.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Krzysztof Kozlowski @ 2024-04-05 9:37 UTC (permalink / raw)
To: Kalle Valo, Bartosz Golaszewski
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <87cyr440hr.fsf@kernel.org>
On 05/04/2024 11:33, Kalle Valo wrote:
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>
>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>
>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> +
>>>> +maintainers:
>>>> + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>
>>> IMHO it would be better to have just driver maintainers listed here.
>>>
>>
>> Why? What's wrong with having the author of the bindings in the Cc list?
>
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
>
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.
This is not a maintainer of driver. This is maintainer of bindings, so
someone who has hardware, datasheets, knowledge and/or interest in
keeping the bindings accurate.
All your arguments above suggest you talk about the driver. This is not
the point here.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: net: bluetooth: btnxpuart: Add firmware-name property
From: Krzysztof Kozlowski @ 2024-04-05 9:39 UTC (permalink / raw)
To: Neeraj Sanjay Kale, marcel, luiz.dentz, davem, edumazet, kuba,
pabeni, robh+dt, krzysztof.kozlowski+dt, conor+dt
Cc: linux-bluetooth, netdev, devicetree, linux-kernel,
amitkumar.karwar, rohit.fule, sherry.sun, ziniu.wang_1,
haibo.chen, LnxRevLi
In-Reply-To: <20240405090118.582310-2-neeraj.sanjaykale@nxp.com>
On 05/04/2024 11:01, Neeraj Sanjay Kale wrote:
> This adds a new optional device tree property called firware-name.
> If this property is specified, the driver overrides the firmware
> name hardcoded in the driver source code.
>
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
> .../devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> index f01a3988538c..25c258212bcd 100644
> --- a/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,88w8987-bt.yaml
> @@ -31,6 +31,13 @@ properties:
> This property depends on the module vendor's
> configuration.
>
> + firmware-name:
> + $ref: /schemas/types.yaml#/definitions/string
drop, missing maxItems. Look at recent code.
> + description:
> + Specify firmware file name, prepended with nxp/.
Why with nxp? What if firmware is stored somewhere else?
> + This property overrides the firmware names hardcoded
> + in the driver source code.
Drop, you now describe current OS policy. What if U-boot treats it
differently?
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Krzysztof Kozlowski @ 2024-04-05 9:41 UTC (permalink / raw)
To: Kalle Valo, Bartosz Golaszewski
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <83dccd7e-e690-4803-adb9-aaedcee7dc94@linaro.org>
On 05/04/2024 11:37, Krzysztof Kozlowski wrote:
> On 05/04/2024 11:33, Kalle Valo wrote:
>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>
>>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>
>>>> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>>>>
>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>>
>>>>> +
>>>>> +maintainers:
>>>>> + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>>>>
>>>> IMHO it would be better to have just driver maintainers listed here.
>>>>
>>>
>>> Why? What's wrong with having the author of the bindings in the Cc list?
>>
>> If you want follow the ath12k development and review patches then you
>> can join the ath12k list. I'm not fond of having too many maintainers,
>> it's not really helping anything and just extra work to periodically
>> cleanup the silent maintainers.
>>
>> I would ask the opposite question: why add you as the maintainer?
>> There's not even a single ath12k patch from you, nor I haven't seen you
>> doing any patch review or otherwise helping others related to ath12k.
>> Don't get me wrong, I value the work you do with this important powerseq
>> feature and hopefully we get it into the tree soon. But I don't see
>> adding you as a maintainer at this point.
>
> This is not a maintainer of driver. This is maintainer of bindings, so
> someone who has hardware, datasheets, knowledge and/or interest in
> keeping the bindings accurate.
>
> All your arguments above suggest you talk about the driver. This is not
> the point here.
>
And to clarify: I do not have opinion whether Bartosz is a suitable
person here and whether driver maintainers should be instead or not. I
only want to clarify the purpose of the binding maintainer.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v6 05/16] dt-bindings: net: wireless: describe the ath12k PCI module
From: Bartosz Golaszewski @ 2024-04-05 9:52 UTC (permalink / raw)
To: Kalle Valo
Cc: Marcel Holtmann, Luiz Augusto von Dentz, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
Liam Girdwood, Mark Brown, Catalin Marinas, Will Deacon,
Bjorn Helgaas, Saravana Kannan, Geert Uytterhoeven, Arnd Bergmann,
Neil Armstrong, Marek Szyprowski, Alex Elder, Srini Kandagatla,
Greg Kroah-Hartman, Abel Vesa, Manivannan Sadhasivam,
Lukas Wunner, Dmitry Baryshkov, linux-bluetooth, netdev,
devicetree, linux-kernel, linux-wireless, linux-arm-msm,
linux-arm-kernel, linux-pci, linux-pm, Bartosz Golaszewski
In-Reply-To: <87cyr440hr.fsf@kernel.org>
On Fri, Apr 5, 2024 at 11:34 AM Kalle Valo <kvalo@kernel.org> wrote:
>
> Bartosz Golaszewski <brgl@bgdev.pl> writes:
>
> > On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Bartosz Golaszewski <brgl@bgdev.pl> writes:
> >>
> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >> >
> >> > +
> >> > +maintainers:
> >> > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >>
> >> IMHO it would be better to have just driver maintainers listed here.
> >>
> >
> > Why? What's wrong with having the author of the bindings in the Cc list?
>
> If you want follow the ath12k development and review patches then you
> can join the ath12k list. I'm not fond of having too many maintainers,
> it's not really helping anything and just extra work to periodically
> cleanup the silent maintainers.
>
> I would ask the opposite question: why add you as the maintainer?
> There's not even a single ath12k patch from you, nor I haven't seen you
> doing any patch review or otherwise helping others related to ath12k.
> Don't get me wrong, I value the work you do with this important powerseq
> feature and hopefully we get it into the tree soon. But I don't see
> adding you as a maintainer at this point.
>
In addition to what Krzysztof already said about you seamingly
confusing the maintenance of the driver vs maintenance of the
device-tree bindings (IOW: structured hardware description) and in
response to your question: I don't see any functional change to any
dt-bindings neither from you nor from Jeff. Are you convinced you can
maintain and properly review any changes?
If so, I don't really care, I can drop myself and have less work.
Bartosz
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox