Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/3] arm64: tlb: Fix TLBI RANGE operand
From: Catalin Marinas @ 2024-04-05 17:10 UTC (permalink / raw)
  To: Gavin Shan
  Cc: linux-arm-kernel, linux-kernel, will, akpm, maz, oliver.upton,
	ryan.roberts, apopple, rananta, mark.rutland, v-songbaohua,
	yangyicong, shahuang, yihyu, shan.gavin
In-Reply-To: <20240405035852.1532010-2-gshan@redhat.com>

On Fri, Apr 05, 2024 at 01:58:50PM +1000, Gavin Shan wrote:
> diff --git a/arch/arm64/include/asm/tlbflush.h b/arch/arm64/include/asm/tlbflush.h
> index 3b0e8248e1a4..a75de2665d84 100644
> --- a/arch/arm64/include/asm/tlbflush.h
> +++ b/arch/arm64/include/asm/tlbflush.h
> @@ -161,12 +161,18 @@ static inline unsigned long get_trans_granule(void)
>  #define MAX_TLBI_RANGE_PAGES		__TLBI_RANGE_PAGES(31, 3)
>  
>  /*
> - * Generate 'num' values from -1 to 30 with -1 rejected by the
> - * __flush_tlb_range() loop below.
> + * Generate 'num' values from -1 to 31 with -1 rejected by the
> + * __flush_tlb_range() loop below. Its return value is only
> + * significant for a maximum of MAX_TLBI_RANGE_PAGES pages. If
> + * 'pages' is more than that, you must iterate over the overall
> + * range.
>   */
> -#define TLBI_RANGE_MASK			GENMASK_ULL(4, 0)
> -#define __TLBI_RANGE_NUM(pages, scale)	\
> -	((((pages) >> (5 * (scale) + 1)) & TLBI_RANGE_MASK) - 1)
> +#define __TLBI_RANGE_NUM(pages, scale)					\
> +	({								\
> +		int __pages = min((pages),				\
> +				  __TLBI_RANGE_PAGES(31, (scale)));	\
> +		(__pages >> (5 * (scale) + 1)) - 1;			\
> +	})

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

This looks correct to me as well. I spent a bit of time to update an old
CBMC model I had around. With the original __TLBI_RANGE_NUM indeed shows
'scale' becoming negative on the kvm_tlb_flush_vmid_range() path. The
patch above fixes it and it also allows the non-KVM path to use the
range TLBI for MAX_TLBI_RANGE_PAGES (as per patch 3).

FWIW, here's the model:

-----------------------8<--------------------------------------
// SPDX-License-Identifier: GPL-2.0-only
/*
 * Check with:
 *   cbmc --unwind 6 tlbinval.c
 */

#define PAGE_SHIFT	(12)
#define PAGE_SIZE	(1 << PAGE_SHIFT)
#define VA_RANGE	(1UL << 48)
#define SZ_64K		0x00010000

#define __round_mask(x, y) ((__typeof__(x))((y)-1))
#define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1)
#define round_down(x, y) ((x) & ~__round_mask(x, y))

#define min(x, y)	(x <= y ? x : y)

#define __ALIGN_KERNEL(x, a)		__ALIGN_KERNEL_MASK(x, (__typeof__(x))(a) - 1)
#define __ALIGN_KERNEL_MASK(x, mask)	(((x) + (mask)) & ~(mask))
#define ALIGN(x, a)			__ALIGN_KERNEL((x), (a))

/* only masking out irrelevant bits */
#define __TLBI_RANGE_VADDR(addr, shift)	((addr) & ~((1UL << shift) - 1))
#define __TLBI_VADDR(addr)		__TLBI_RANGE_VADDR(addr, PAGE_SHIFT)

#define __TLBI_RANGE_PAGES(num, scale)	((unsigned long)((num) + 1) << (5 * (scale) + 1))
#define MAX_TLBI_RANGE_PAGES		__TLBI_RANGE_PAGES(31, 3)

#if 0
/* original code */
#define TLBI_RANGE_MASK			0x1fUL
#define __TLBI_RANGE_NUM(pages, scale)	\
	((((int)(pages) >> (5 * (scale) + 1)) & TLBI_RANGE_MASK) - 1)
#else
#define __TLBI_RANGE_NUM(pages, scale)					\
	({								\
		int __pages = min((pages),				\
				  __TLBI_RANGE_PAGES(31, (scale)));	\
		(__pages >> (5 * (scale) + 1)) - 1;			\
	})
#endif

const static _Bool lpa2 = 1;
const static _Bool kvm = 1;

static unsigned long inval_start;
static unsigned long inval_end;

static void tlbi(unsigned long start, unsigned long size)
{
	unsigned long end = start + size;

	if (inval_end == 0) {
		inval_start = start;
		inval_end = end;
		return;
	}

	/* optimal invalidation */
	__CPROVER_assert(start >= inval_end || end <= inval_start, "No overlapping TLBI range");

	if (start < inval_start) {
		__CPROVER_assert(end >= inval_start, "No TLBI range gaps");
		inval_start = start;
	}
	if (end > inval_end) {
		__CPROVER_assert(start <= inval_end, "No TLBI range gaps");
		inval_end = end;
	}
}

static void tlbi_range(unsigned long start, int num, int scale)
{
	unsigned long size = __TLBI_RANGE_PAGES(num, scale) << PAGE_SHIFT;

	tlbi(start, size);
}

static void __flush_tlb_range_op(unsigned long start, unsigned long pages,
				 unsigned long stride)
{
	int num = 0;
	int scale = 3;
	int shift = lpa2 ? 16 : PAGE_SHIFT;
	unsigned long addr;

	while (pages > 0) {
		if (pages == 1 ||
		    (lpa2 && start != ALIGN(start, SZ_64K))) {
			addr = __TLBI_VADDR(start);
			tlbi(addr, stride);
			start += stride;
			pages -= stride >> PAGE_SHIFT;
			continue;
		}

		__CPROVER_assert(scale >= 0 && scale <= 3, "Scale in range");
		num = __TLBI_RANGE_NUM(pages, scale);
		__CPROVER_assert(num <= 31, "Num in range");
		if (num >= 0) {
			addr = __TLBI_RANGE_VADDR(start, shift);
			tlbi_range(addr, num, scale);
			start += __TLBI_RANGE_PAGES(num, scale) << PAGE_SHIFT;
			pages -= __TLBI_RANGE_PAGES(num, scale);
		}
		scale--;
	}
}

static void __flush_tlb_range(unsigned long start, unsigned long pages,
			      unsigned long stride)
{
	if (pages > MAX_TLBI_RANGE_PAGES) {
		tlbi(0, VA_RANGE);
		return;
	}

	__flush_tlb_range_op(start, pages, stride);
}

void __kvm_tlb_flush_vmid_range(unsigned long start, unsigned long pages)
{
	unsigned long stride;

	stride = PAGE_SIZE;
	start = round_down(start, stride);

	__flush_tlb_range_op(start, pages, stride);
}

static void kvm_tlb_flush_vmid_range(unsigned long addr, unsigned long size)
{
	unsigned long pages, inval_pages;

	pages = size >> PAGE_SHIFT;
	while (pages > 0) {
		inval_pages = min(pages, MAX_TLBI_RANGE_PAGES);
		__kvm_tlb_flush_vmid_range(addr, inval_pages);

		addr += inval_pages << PAGE_SHIFT;
		pages -= inval_pages;
	}
}

static unsigned long nondet_ulong(void);

int main(void)
{
	unsigned long stride = nondet_ulong();
	unsigned long start = round_down(nondet_ulong(), stride);
	unsigned long end = round_up(nondet_ulong(), stride);
	unsigned long pages = (end - start) >> PAGE_SHIFT;

	__CPROVER_assume(stride == PAGE_SIZE ||
			 stride == PAGE_SIZE << (PAGE_SHIFT - 3) ||
			 stride == PAGE_SIZE << (2 * (PAGE_SHIFT - 3)));
	__CPROVER_assume(start < end);
	__CPROVER_assume(end <= VA_RANGE);

	if (kvm)
		kvm_tlb_flush_vmid_range(start, pages << PAGE_SHIFT);
	else
		__flush_tlb_range(start, pages, stride);

	__CPROVER_assert((inval_start == 0 && inval_end == VA_RANGE) ||
			 (inval_start == start && inval_end == end),
			 "Correct invalidation");

	return 0;
}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] phy: xilinx: Convert to platform remove callback returning void
From: Vinod Koul @ 2024-04-05 17:09 UTC (permalink / raw)
  To: Laurent Pinchart, Kishon Vijay Abraham I, Michal Simek,
	Uwe Kleine-König
  Cc: linux-kernel, linux-phy, linux-arm-kernel, kernel
In-Reply-To: <57a3338a1cec683ac84d48e00dbf197e15ee5481.1709886922.git.u.kleine-koenig@pengutronix.de>


On Fri, 08 Mar 2024 09:51:13 +0100, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource leaks.
> 
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
> 
> [...]

Applied, thanks!

[1/1] phy: xilinx: Convert to platform remove callback returning void
      commit: 7dcb8668aedc5603cba1f2625c6051beff03797d

Best regards,
-- 
~Vinod



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] phy: rockchip: Fix typo in function names
From: Vinod Koul @ 2024-04-05 17:09 UTC (permalink / raw)
  To: rick.wertenbroek, Rick Wertenbroek
  Cc: Kishon Vijay Abraham I, Heiko Stuebner, linux-phy,
	linux-arm-kernel, linux-rockchip, linux-kernel
In-Reply-To: <20240307095318.3651498-1-rick.wertenbroek@gmail.com>


On Thu, 07 Mar 2024 10:53:18 +0100, Rick Wertenbroek wrote:
> Several functions had "rochchip" instead of "rockchip" in their name.
> Replace "rochchip" by "rockchip".
> 
> 

Applied, thanks!

[1/1] phy: rockchip: Fix typo in function names
      commit: 9b6bfad9070a95d19973be17177e5d9220cbbf1f

Best regards,
-- 
~Vinod



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] iommu/arm-smmu-v3: Retire disable_bypass parameter
From: Robin Murphy @ 2024-04-05 16:52 UTC (permalink / raw)
  To: will, joro; +Cc: iommu, linux-arm-kernel

The disable_bypass parameter has been mostly meaningless for a long time
since the introduction of default domains. Its original intent is now
fulfilled by the controls users have over the default domain type, and
its remaining effect in the brief window between Stream Table
initialisation and default domain creation hardly seems worth the
complication. Furthermore, thanks to 2-level Stream Tables, disabling
disable_bypass (there's another reason not to like it right there) has
never guaranteed that any particular StreamID *will* bypass anyway - any
device which might actually care about that wants RMRs - so there's not
really much lost by taking away that option (which has already been
non-default for nearing 6 years now).

As part of this, also remove the weird behaviour where we "successfully"
probe and register a non-functional SMMU if the DT "#iommu-cells"
property is wrong. I have no memory of what possessed me to think that
was a good idea at the time, and by now I suspect it's likely to break
things worse than simply failing probe would.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 46 ++++++---------------
 1 file changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 41f93c3ab160..4eb74f0ad13b 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -30,11 +30,6 @@
 #include "arm-smmu-v3.h"
 #include "../../dma-iommu.h"
 
-static bool disable_bypass = true;
-module_param(disable_bypass, bool, 0444);
-MODULE_PARM_DESC(disable_bypass,
-	"Disable bypass streams such that incoming transactions from devices that are not attached to an iommu domain will report an abort back to the device and will not be allowed to pass through the SMMU.");
-
 static bool disable_msipolling;
 module_param(disable_msipolling, bool, 0444);
 MODULE_PARM_DESC(disable_msipolling,
@@ -1567,17 +1562,13 @@ static void arm_smmu_make_s2_domain_ste(struct arm_smmu_ste *target,
  * This can safely directly manipulate the STE memory without a sync sequence
  * because the STE table has not been installed in the SMMU yet.
  */
-static void arm_smmu_init_initial_stes(struct arm_smmu_device *smmu,
-				       struct arm_smmu_ste *strtab,
+static void arm_smmu_init_initial_stes(struct arm_smmu_ste *strtab,
 				       unsigned int nent)
 {
 	unsigned int i;
 
 	for (i = 0; i < nent; ++i) {
-		if (disable_bypass)
-			arm_smmu_make_abort_ste(strtab);
-		else
-			arm_smmu_make_bypass_ste(smmu, strtab);
+		arm_smmu_make_abort_ste(strtab);
 		strtab++;
 	}
 }
@@ -1605,7 +1596,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 		return -ENOMEM;
 	}
 
-	arm_smmu_init_initial_stes(smmu, desc->l2ptr, 1 << STRTAB_SPLIT);
+	arm_smmu_init_initial_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
 	arm_smmu_write_strtab_l1_desc(strtab, desc);
 	return 0;
 }
@@ -2915,10 +2906,10 @@ static void arm_smmu_release_device(struct device *dev)
 		iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
 
 	/* Put the STE back to what arm_smmu_init_strtab() sets */
-	if (disable_bypass && !dev->iommu->require_direct)
-		arm_smmu_attach_dev_blocked(&arm_smmu_blocked_domain, dev);
-	else
+	if (dev->iommu->require_direct)
 		arm_smmu_attach_dev_identity(&arm_smmu_identity_domain, dev);
+	else
+		arm_smmu_attach_dev_blocked(&arm_smmu_blocked_domain, dev);
 
 	arm_smmu_disable_pasid(master);
 	arm_smmu_remove_master(master);
@@ -3273,7 +3264,7 @@ static int arm_smmu_init_strtab_linear(struct arm_smmu_device *smmu)
 	reg |= FIELD_PREP(STRTAB_BASE_CFG_LOG2SIZE, smmu->sid_bits);
 	cfg->strtab_base_cfg = reg;
 
-	arm_smmu_init_initial_stes(smmu, strtab, cfg->num_l1_ents);
+	arm_smmu_init_initial_stes(strtab, cfg->num_l1_ents);
 	return 0;
 }
 
@@ -3503,7 +3494,7 @@ static int arm_smmu_device_disable(struct arm_smmu_device *smmu)
 	return ret;
 }
 
-static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
+static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
 {
 	int ret;
 	u32 reg, enables;
@@ -3513,7 +3504,6 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 	reg = readl_relaxed(smmu->base + ARM_SMMU_CR0);
 	if (reg & CR0_SMMUEN) {
 		dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n");
-		WARN_ON(is_kdump_kernel() && !disable_bypass);
 		arm_smmu_update_gbpa(smmu, GBPA_ABORT, 0);
 	}
 
@@ -3620,14 +3610,8 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)
 	if (is_kdump_kernel())
 		enables &= ~(CR0_EVTQEN | CR0_PRIQEN);
 
-	/* Enable the SMMU interface, or ensure bypass */
-	if (!bypass || disable_bypass) {
-		enables |= CR0_SMMUEN;
-	} else {
-		ret = arm_smmu_update_gbpa(smmu, 0, GBPA_ABORT);
-		if (ret)
-			return ret;
-	}
+	/* Enable the SMMU interface */
+	enables |= CR0_SMMUEN;
 	ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
 				      ARM_SMMU_CR0ACK);
 	if (ret) {
@@ -4019,7 +4003,6 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	resource_size_t ioaddr;
 	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
-	bool bypass;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
 	if (!smmu)
@@ -4030,12 +4013,9 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 		ret = arm_smmu_device_dt_probe(pdev, smmu);
 	} else {
 		ret = arm_smmu_device_acpi_probe(pdev, smmu);
-		if (ret == -ENODEV)
-			return ret;
 	}
-
-	/* Set bypass mode according to firmware probing result */
-	bypass = !!ret;
+	if (ret)
+		return ret;
 
 	/* Base address */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -4099,7 +4079,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
 	arm_smmu_rmr_install_bypass_ste(smmu);
 
 	/* Reset the device */
-	ret = arm_smmu_device_reset(smmu, bypass);
+	ret = arm_smmu_device_reset(smmu);
 	if (ret)
 		return ret;
 
-- 
2.39.2.101.g768bb238c484.dirty


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: [PATCH v1 4/4] arm64: dts: freescale: imx8mm-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
	linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-5-eichest@gmail.com>

On Fri, Apr 05, 2024 at 06:07:20PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 3/4] arm64: dts: freescale: imx8mm-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
	linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-4-eichest@gmail.com>

On Fri, Apr 05, 2024 at 06:07:19PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 2/4] arm64: dts: freescale: imx8mp-verdin-dahlia: support sleep-moci
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
	linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-3-eichest@gmail.com>

On Fri, Apr 05, 2024 at 06:07:18PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> Previously, we had the sleep-moci pin set to always on. However, the
> Dahlia carrier board supports disabling the sleep-moci when the system
> is suspended to power down peripherals that support it. This reduces
> overall power consumption. This commit adds support for this feature by
> disabling the reg_force_sleep_moci regulator and adding two new
> regulators for the USB hub and PCIe that can be turned off when the
> system is suspended.
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 1/4] arm64: dts: freescale: imx8mp-verdin: replace sleep-moci hog with regulator
From: Francesco Dolcini @ 2024-04-05 16:48 UTC (permalink / raw)
  To: Stefan Eichenberger
  Cc: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini, devicetree, imx, linux-arm-kernel,
	linux-kernel, Stefan Eichenberger
In-Reply-To: <20240405160720.5977-2-eichest@gmail.com>

On Fri, Apr 05, 2024 at 06:07:17PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> The Verdin family has a signal called sleep-moci which can be used to
> turn off peripherals on the carrier board when the SoM goes into
> suspend. So far we have hogged this signal, which means the peripherals
> are always on and it is not possible to add peripherals that depend on
> the sleep-moci to be on. With this change, we replace the hog with a
> regulator so that peripherals can add their own regulators that use the
> same gpio. Carrier boards that allow peripherals to be powered off in
> suspend can disable this regulator and implement their own regulator to
> control the sleep-moci.
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
From: Marco Felsch @ 2024-04-05 16:44 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Sudeep Holla, Cristian Marussi, devicetree, Peng Fan,
	linux-kernel, linux-arm-kernel, imx
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-4-9fc9186856c2@nxp.com>

Hi Peng,

On 24-04-05, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> The i.MX MISC protocol is for misc settings, such as gpio expander
> wakeup.

Can you elaborate a bit more please?

Regards,
  Marco


> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/firmware/arm_scmi/Kconfig       |  10 ++
>  drivers/firmware/arm_scmi/Makefile      |   1 +
>  drivers/firmware/arm_scmi/imx-sm-misc.c | 305 ++++++++++++++++++++++++++++++++
>  include/linux/scmi_imx_protocol.h       |  17 ++
>  4 files changed, 333 insertions(+)
> 
> diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
> index 56d11c9d9f47..bfeae92f6420 100644
> --- a/drivers/firmware/arm_scmi/Kconfig
> +++ b/drivers/firmware/arm_scmi/Kconfig
> @@ -191,3 +191,13 @@ config IMX_SCMI_BBM_EXT
>  	  and BUTTON.
>  
>  	  This driver can also be built as a module.
> +
> +config IMX_SCMI_MISC_EXT
> +	tristate "i.MX SCMI MISC EXTENSION"
> +	depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
> +	default y if ARCH_MXC
> +	help
> +	  This enables i.MX System MISC control logic such as gpio expander
> +	  wakeup
> +
> +	  This driver can also be built as a module.
> diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
> index 327687acf857..a23fde721222 100644
> --- a/drivers/firmware/arm_scmi/Makefile
> +++ b/drivers/firmware/arm_scmi/Makefile
> @@ -12,6 +12,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
>  scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
>  scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
>  scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
> +scmi-protocols-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
>  scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
>  
>  obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o
> diff --git a/drivers/firmware/arm_scmi/imx-sm-misc.c b/drivers/firmware/arm_scmi/imx-sm-misc.c
> new file mode 100644
> index 000000000000..1b0ec2281518
> --- /dev/null
> +++ b/drivers/firmware/arm_scmi/imx-sm-misc.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * System control and Management Interface (SCMI) NXP MISC Protocol
> + *
> + * Copyright 2024 NXP
> + */
> +
> +#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
> +
> +#include <linux/bits.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/scmi_protocol.h>
> +#include <linux/scmi_imx_protocol.h>
> +
> +#include "protocols.h"
> +#include "notify.h"
> +
> +#define SCMI_PROTOCOL_SUPPORTED_VERSION		0x10000
> +
> +enum scmi_imx_misc_protocol_cmd {
> +	SCMI_IMX_MISC_CTRL_SET	= 0x3,
> +	SCMI_IMX_MISC_CTRL_GET	= 0x4,
> +	SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
> +};
> +
> +struct scmi_imx_misc_info {
> +	u32 version;
> +	u32 nr_dev_ctrl;
> +	u32 nr_brd_ctrl;
> +	u32 nr_reason;
> +};
> +
> +struct scmi_msg_imx_misc_protocol_attributes {
> +	__le32 attributes;
> +};
> +
> +#define GET_BRD_CTRLS_NR(x)	le32_get_bits((x), GENMASK(31, 24))
> +#define GET_REASONS_NR(x)	le32_get_bits((x), GENMASK(23, 16))
> +#define GET_DEV_CTRLS_NR(x)	le32_get_bits((x), GENMASK(15, 0))
> +#define BRD_CTRL_START_ID	BIT(15)
> +
> +struct scmi_imx_misc_ctrl_set_in {
> +	__le32 id;
> +	__le32 num;
> +	__le32 value[MISC_MAX_VAL];
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_in {
> +	__le32 ctrl_id;
> +	__le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_notify_payld {
> +	__le32 ctrl_id;
> +	__le32 flags;
> +};
> +
> +struct scmi_imx_misc_ctrl_get_out {
> +	__le32 num;
> +	__le32 *val;
> +};
> +
> +static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle *ph,
> +					struct scmi_imx_misc_info *mi)
> +{
> +	int ret;
> +	struct scmi_xfer *t;
> +	struct scmi_msg_imx_misc_protocol_attributes *attr;
> +
> +	ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
> +				      sizeof(*attr), &t);
> +	if (ret)
> +		return ret;
> +
> +	attr = t->rx.buf;
> +
> +	ret = ph->xops->do_xfer(ph, t);
> +	if (!ret) {
> +		mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
> +		mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
> +		mi->nr_reason = GET_REASONS_NR(attr->attributes);
> +		dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM BRD CTRL: %d,NUM Reason: %d\n",
> +			 mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
> +	}
> +
> +	ph->xops->xfer_put(ph, t);
> +
> +	return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_validate_id(const struct scmi_protocol_handle *ph,
> +					  u32 ctrl_id)
> +{
> +	struct scmi_imx_misc_info *mi = ph->get_priv(ph);
> +
> +	if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
> +		return -EINVAL;
> +	if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle *ph,
> +				     u32 ctrl_id, u32 flags)
> +{
> +	struct scmi_imx_misc_ctrl_notify_in *in;
> +	struct scmi_xfer *t;
> +	int ret;
> +
> +	ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> +	if (ret)
> +		return ret;
> +
> +	ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
> +				      sizeof(*in), 0, &t);
> +	if (ret)
> +		return ret;
> +
> +	in = t->tx.buf;
> +	in->ctrl_id = cpu_to_le32(ctrl_id);
> +	in->flags = cpu_to_le32(flags);
> +
> +	ret = ph->xops->do_xfer(ph, t);
> +
> +	ph->xops->xfer_put(ph, t);
> +
> +	return ret;
> +}
> +
> +static int
> +scmi_imx_misc_ctrl_set_notify_enabled(const struct scmi_protocol_handle *ph,
> +				      u8 evt_id, u32 src_id, bool enable)
> +{
> +	int ret;
> +
> +	ret = scmi_imx_misc_ctrl_notify(ph, src_id, enable ? evt_id : 0);
> +	if (ret)
> +		dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] - ret:%d\n",
> +			evt_id, src_id, ret);
> +
> +	return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_get_num_sources(const struct scmi_protocol_handle *ph)
> +{
> +	return GENMASK(15, 0);
> +}
> +
> +static void *
> +scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle *ph,
> +				      u8 evt_id, ktime_t timestamp,
> +				      const void *payld, size_t payld_sz,
> +				      void *report, u32 *src_id)
> +{
> +	const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
> +	struct scmi_imx_misc_ctrl_notify_report *r = report;
> +
> +	if (sizeof(*p) != payld_sz)
> +		return NULL;
> +
> +	r->timestamp = timestamp;
> +	r->ctrl_id = p->ctrl_id;
> +	r->flags = p->flags;
> +	*src_id = r->ctrl_id;
> +	dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
> +		r->ctrl_id, r->flags);
> +
> +	return r;
> +}
> +
> +static const struct scmi_event_ops scmi_imx_misc_event_ops = {
> +	.get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
> +	.set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
> +	.fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
> +};
> +
> +static const struct scmi_event scmi_imx_misc_events[] = {
> +	{
> +		.id = SCMI_EVENT_IMX_MISC_CONTROL_DISABLED,
> +		.max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> +		.max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> +	},
> +	{
> +		.id = SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE,
> +		.max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> +		.max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> +	},
> +	{
> +		.id = SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE,
> +		.max_payld_sz = sizeof(struct scmi_imx_misc_ctrl_notify_payld),
> +		.max_report_sz = sizeof(struct scmi_imx_misc_ctrl_notify_report),
> +	}
> +};
> +
> +static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
> +	.queue_sz = SCMI_PROTO_QUEUE_SZ,
> +	.ops = &scmi_imx_misc_event_ops,
> +	.evts = scmi_imx_misc_events,
> +	.num_events = ARRAY_SIZE(scmi_imx_misc_events),
> +};
> +
> +static int scmi_imx_misc_protocol_init(const struct scmi_protocol_handle *ph)
> +{
> +	struct scmi_imx_misc_info *minfo;
> +	u32 version;
> +	int ret;
> +
> +	ret = ph->xops->version_get(ph, &version);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
> +		 PROTOCOL_REV_MAJOR(version), PROTOCOL_REV_MINOR(version));
> +
> +	minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
> +	if (!minfo)
> +		return -ENOMEM;
> +
> +	ret = scmi_imx_misc_attributes_get(ph, minfo);
> +	if (ret)
> +		return ret;
> +
> +	return ph->set_priv(ph, minfo, version);
> +}
> +
> +static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> +				  u32 ctrl_id, u32 *num, u32 *val)
> +{
> +	struct scmi_imx_misc_ctrl_get_out *out;
> +	struct scmi_xfer *t;
> +	int ret, i;
> +
> +	ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> +	if (ret)
> +		return ret;
> +
> +	ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET, sizeof(u32),
> +				      0, &t);
> +	if (ret)
> +		return ret;
> +
> +	put_unaligned_le32(ctrl_id, t->tx.buf);
> +	ret = ph->xops->do_xfer(ph, t);
> +	if (!ret) {
> +		out = t->rx.buf;
> +		*num = le32_to_cpu(out->num);
> +		for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
> +			val[i] = le32_to_cpu(out->val[i]);
> +	}
> +
> +	ph->xops->xfer_put(ph, t);
> +
> +	return ret;
> +}
> +
> +static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
> +				  u32 ctrl_id, u32 num, u32 *val)
> +{
> +	struct scmi_imx_misc_ctrl_set_in *in;
> +	struct scmi_xfer *t;
> +	int ret, i;
> +
> +	ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> +	if (ret)
> +		return ret;
> +
> +	if (num > MISC_MAX_VAL)
> +		return -EINVAL;
> +
> +	ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET, sizeof(*in),
> +				      0, &t);
> +	if (ret)
> +		return ret;
> +
> +	in = t->tx.buf;
> +	in->id = cpu_to_le32(ctrl_id);
> +	in->num = cpu_to_le32(num);
> +	for (i = 0; i < num; i++)
> +		in->value[i] = cpu_to_le32(val[i]);
> +
> +	ret = ph->xops->do_xfer(ph, t);
> +
> +	ph->xops->xfer_put(ph, t);
> +
> +	return ret;
> +}
> +
> +static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops = {
> +	.misc_ctrl_set = scmi_imx_misc_ctrl_set,
> +	.misc_ctrl_get = scmi_imx_misc_ctrl_get,
> +};
> +
> +static const struct scmi_protocol scmi_imx_misc = {
> +	.id = SCMI_PROTOCOL_IMX_MISC,
> +	.owner = THIS_MODULE,
> +	.instance_init = &scmi_imx_misc_protocol_init,
> +	.ops = &scmi_imx_misc_proto_ops,
> +	.events = &scmi_imx_misc_protocol_events,
> +	.supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION,
> +};
> +module_scmi_protocol(scmi_imx_misc);
> diff --git a/include/linux/scmi_imx_protocol.h b/include/linux/scmi_imx_protocol.h
> index 90ce011a4429..a69bd4a20f0f 100644
> --- a/include/linux/scmi_imx_protocol.h
> +++ b/include/linux/scmi_imx_protocol.h
> @@ -13,8 +13,14 @@
>  #include <linux/notifier.h>
>  #include <linux/types.h>
>  
> +#define SCMI_PAYLOAD_LEN	100
> +
> +#define SCMI_ARRAY(X, Y)	((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
> +#define MISC_MAX_VAL		SCMI_ARRAY(8, uint32_t)
> +
>  enum scmi_nxp_protocol {
>  	SCMI_PROTOCOL_IMX_BBM = 0x81,
> +	SCMI_PROTOCOL_IMX_MISC = 0x84,
>  };
>  
>  struct scmi_imx_bbm_proto_ops {
> @@ -42,4 +48,15 @@ struct scmi_imx_bbm_notif_report {
>  	unsigned int		rtc_id;
>  	unsigned int		rtc_evt;
>  };
> +
> +struct scmi_imx_misc_ctrl_notify_report {
> +	ktime_t			timestamp;
> +	unsigned int		ctrl_id;
> +	unsigned int		flags;
> +};
> +
> +struct scmi_imx_misc_proto_ops {
> +	int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id, u32 num, u32 *val);
> +	int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id, u32 *num, u32 *val);
> +};
>  #endif
> 
> -- 
> 2.37.1
> 
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 08/18] phy: ti: phy-j721e-wiz: add resume support
From: Vinod Koul @ 2024-04-05 16:44 UTC (permalink / raw)
  To: Thomas Richard
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Tony Lindgren, Haojian Zhuang, Vignesh R, Aaro Koskinen,
	Janusz Krzysztofik, Andi Shyti, Peter Rosin,
	Kishon Vijay Abraham I, Philipp Zabel, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, linux-gpio,
	linux-kernel, linux-arm-kernel, linux-omap, linux-i2c, linux-phy,
	linux-pci, gregory.clement, theo.lebrun, thomas.petazzoni,
	u-kumar1
In-Reply-To: <20240102-j7200-pcie-s2r-v4-8-6f1f53390c85@bootlin.com>

On 04-03-24, 16:35, Thomas Richard wrote:
> Add resume support.
> It has been tested on J7200 SR1.0 and SR2.0.
> 
> Co-developed-by: Théo Lebrun <theo.lebrun@bootlin.com>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>  drivers/phy/ti/phy-j721e-wiz.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index 0e3cb1ed5a52..b2320f2efb72 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1660,12 +1660,41 @@ static void wiz_remove(struct platform_device *pdev)
>  	pm_runtime_disable(dev);
>  }
>  
> +static int wiz_resume_noirq(struct device *dev)

I think this should be annotated with __maybe_unused

> +{
> +	struct device_node *node = dev->of_node;
> +	struct wiz *wiz = dev_get_drvdata(dev);
> +	int ret;
> +
> +	/* Enable supplemental Control override if available */
> +	if (wiz->sup_legacy_clk_override)
> +		regmap_field_write(wiz->sup_legacy_clk_override, 1);
> +
> +	wiz_clock_init(wiz);
> +
> +	ret = wiz_init(wiz);
> +	if (ret) {
> +		dev_err(dev, "WIZ initialization failed\n");
> +		goto err_wiz_init;
> +	}
> +
> +	return 0;
> +
> +err_wiz_init:
> +	wiz_clock_cleanup(wiz, node);
> +
> +	return ret;
> +}
> +
> +static DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, NULL, wiz_resume_noirq);
> +
>  static struct platform_driver wiz_driver = {
>  	.probe		= wiz_probe,
>  	.remove_new	= wiz_remove,
>  	.driver		= {
>  		.name	= "wiz",
>  		.of_match_table = wiz_id_table,
> +		.pm	= pm_sleep_ptr(&wiz_pm_ops),
>  	},
>  };
>  module_platform_driver(wiz_driver);
> 
> -- 
> 2.39.2

-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 07/18] phy: ti: phy-j721e-wiz: split wiz_clock_init() function
From: Vinod Koul @ 2024-04-05 16:42 UTC (permalink / raw)
  To: Thomas Richard
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Tony Lindgren, Haojian Zhuang, Vignesh R, Aaro Koskinen,
	Janusz Krzysztofik, Andi Shyti, Peter Rosin,
	Kishon Vijay Abraham I, Philipp Zabel, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas, linux-gpio,
	linux-kernel, linux-arm-kernel, linux-omap, linux-i2c, linux-phy,
	linux-pci, gregory.clement, theo.lebrun, thomas.petazzoni,
	u-kumar1
In-Reply-To: <20240102-j7200-pcie-s2r-v4-7-6f1f53390c85@bootlin.com>

On 04-03-24, 16:35, Thomas Richard wrote:
> The wiz_clock_init() function mixes probe and hardware configuration.
> Rename the wiz_clock_init() to wiz_clock_probe() and move the hardware
> configuration part in a new function named wiz_clock_init().
> 
> This hardware configuration sequence must be called during the resume
> stage of the driver.

Do you have phy patches dependent on rest, if not consider submitting
them in a separate series

> 
> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com>
> ---
>  drivers/phy/ti/phy-j721e-wiz.c | 67 ++++++++++++++++++++++++------------------
>  1 file changed, 38 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index 5fea4df9404e..0e3cb1ed5a52 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -1076,26 +1076,12 @@ static int wiz_clock_register(struct wiz *wiz)
>  	return ret;
>  }
>  
> -static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
> +static void wiz_clock_init(struct wiz *wiz)
>  {
> -	const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
> -	struct device *dev = wiz->dev;
> -	struct device_node *clk_node;
> -	const char *node_name;
>  	unsigned long rate;
> -	struct clk *clk;
> -	int ret;
> -	int i;
> -
> -	clk = devm_clk_get(dev, "core_ref_clk");
> -	if (IS_ERR(clk))
> -		return dev_err_probe(dev, PTR_ERR(clk),
> -				     "core_ref_clk clock not found\n");
>  
> -	wiz->input_clks[WIZ_CORE_REFCLK] = clk;
> -
> -	rate = clk_get_rate(clk);
> -	if (rate >= 100000000)
> +	rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK]);
> +	if (rate >= REF_CLK_100MHZ)
>  		regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x1);
>  	else
>  		regmap_field_write(wiz->pma_cmn_refclk_int_mode, 0x3);
> @@ -1119,6 +1105,39 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
>  		break;
>  	}
>  
> +	if (wiz->input_clks[WIZ_CORE_REFCLK1]) {
> +		rate = clk_get_rate(wiz->input_clks[WIZ_CORE_REFCLK1]);
> +		if (rate >= REF_CLK_100MHZ)
> +			regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
> +		else
> +			regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
> +

unnecessary empty line

> +	}
> +
> +	rate = clk_get_rate(wiz->input_clks[WIZ_EXT_REFCLK]);
> +	if (rate >= REF_CLK_100MHZ)
> +		regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
> +	else
> +		regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
> +}
> +
> +static int wiz_clock_probe(struct wiz *wiz, struct device_node *node)
> +{
> +	const struct wiz_clk_mux_sel *clk_mux_sel = wiz->clk_mux_sel;
> +	struct device *dev = wiz->dev;
> +	struct device_node *clk_node;
> +	const char *node_name;
> +	struct clk *clk;
> +	int ret;
> +	int i;
> +
> +	clk = devm_clk_get(dev, "core_ref_clk");
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "core_ref_clk clock not found\n");
> +
> +	wiz->input_clks[WIZ_CORE_REFCLK] = clk;
> +
>  	if (wiz->data->pma_cmn_refclk1_int_mode) {
>  		clk = devm_clk_get(dev, "core_ref1_clk");
>  		if (IS_ERR(clk))
> @@ -1126,12 +1145,6 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
>  					     "core_ref1_clk clock not found\n");
>  
>  		wiz->input_clks[WIZ_CORE_REFCLK1] = clk;
> -
> -		rate = clk_get_rate(clk);
> -		if (rate >= 100000000)
> -			regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x1);
> -		else
> -			regmap_field_write(wiz->pma_cmn_refclk1_int_mode, 0x3);
>  	}
>  
>  	clk = devm_clk_get(dev, "ext_ref_clk");
> @@ -1141,11 +1154,7 @@ static int wiz_clock_init(struct wiz *wiz, struct device_node *node)
>  
>  	wiz->input_clks[WIZ_EXT_REFCLK] = clk;
>  
> -	rate = clk_get_rate(clk);
> -	if (rate >= 100000000)
> -		regmap_field_write(wiz->pma_cmn_refclk_mode, 0x0);
> -	else
> -		regmap_field_write(wiz->pma_cmn_refclk_mode, 0x2);
> +	wiz_clock_init(wiz);
>  
>  	switch (wiz->type) {
>  	case AM64_WIZ_10G:
> @@ -1589,7 +1598,7 @@ static int wiz_probe(struct platform_device *pdev)
>  		goto err_get_sync;
>  	}
>  
> -	ret = wiz_clock_init(wiz, node);
> +	ret = wiz_clock_probe(wiz, node);
>  	if (ret < 0) {
>  		dev_warn(dev, "Failed to initialize clocks\n");
>  		goto err_get_sync;
> 
> -- 
> 2.39.2

-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v3 18/25] dt-bindings: media: imx258: Add alternate compatible strings
From: Conor Dooley @ 2024-04-05 16:24 UTC (permalink / raw)
  To: Dave Stevenson
  Cc: git, linux-media, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
	linux-kernel, pavel, phone-devel
In-Reply-To: <20240405-affair-cruelly-a7e9d23b597c@spud>


[-- Attachment #1.1: Type: text/plain, Size: 2677 bytes --]

On Fri, Apr 05, 2024 at 05:24:11PM +0100, Conor Dooley wrote:
> On Fri, Apr 05, 2024 at 11:25:50AM +0100, Dave Stevenson wrote:
> > Hi Conor
> > 
> > On Wed, 3 Apr 2024 at 17:14, Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Wed, Apr 03, 2024 at 09:03:47AM -0600, git@luigi311.com wrote:
> > > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > >
> > > > There are a number of variants of the imx258 modules that can not
> > > > be differentiated at runtime, so add compatible strings for the
> > > > PDAF variant.
> > > >
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > > Signed-off-by: Luis Garcia <git@luigi311.com>
> > > > ---
> > > >  .../devicetree/bindings/media/i2c/sony,imx258.yaml       | 9 +++++++--
> > > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > index bee61a443b23..c978abc0cdb3 100644
> > > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > > @@ -13,11 +13,16 @@ description: |-
> > > >    IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > > >    type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > > >    is programmable through I2C interface.  Image data is sent through MIPI
> > > > -  CSI-2.
> > > > +  CSI-2. The sensor exists in two different models, a standard variant
> > > > +  (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> > > > +  The camera module does not expose the model through registers, so the
> > > > +  exact model needs to be specified.
> > > >
> > > >  properties:
> > > >    compatible:
> > > > -    const: sony,imx258
> > > > +    enum:
> > > > +      - sony,imx258
> > > > +      - sony,imx258-pdaf
> > >
> > > Does the pdaf variant support all of the features/is it register
> > > compatible with the regular variant? If it is, the regular variant
> > > should be a fallback compatible.
> > 
> > It has the same register set, but certain registers have to be
> > programmed differently so that the image is corrected for the
> > partially shielded pixels used for phase detect auto focus (PDAF).
> > Either compatible will "work" on either variant of the module, but
> > you'll get weird image artifacts when using the wrong one.
> 
> To paraphase, a fallback compatible is not suitable.

Whoops, I forgot this:
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: iMX8M Mini suspend/resume hanging on imx8m_blk_ctrl_power_on()
From: vitor @ 2024-04-05 15:06 UTC (permalink / raw)
  To: linux-pm, imx, linux-arm-kernel, linux-kernel
  Cc: vitor.soares, ulf.hansson, shawnguo, s.hauer, kernel, festevam,
	rafael, geert+renesas, peng.fan, linus.walleij, u.kleine-koenig,
	marex
In-Reply-To: <fccbb040330a706a4f7b34875db1d896a0bf81c8.camel@gmail.com>

Hi,

On Thu, 2024-04-04 at 16:53 +0100, vitor wrote:
> Greetings,
> 
> I'm trying to suspend/resume our Verdin iMX8M Mini with VPU IP using
> the latest 6.9.0-rc2 Kernel. While the system can suspend without
> issues, it hangs on the resume routine. After some investigation, I
> can
> see the Kernel hanging on imx8m_blk_ctrl_power_on()[1] while resuming
> the hantro-vpu power domain.
> 
> Any hint about that?
> 
> [1]
> https://elixir.bootlin.com/linux/v6.9-rc2/source/drivers/pmdomain/imx
> /imx8m-blk-ctrl.c#L101
> 

Looking at other child nodes of the pgc node, pgc_vpu_[g1|g2|h1] seems
to be nested into pgc_vpumix.

After applying the following changes to imx8mm.dtsi, the suspend/resume
is working.


@@ -739,16 +739,19 @@ pgc_vpumix: power-domain@6 {
	pgc_vpu_g1: power-domain@7 {
		#power-domain-cells = <0>;
		reg = <IMX8MM_POWER_DOMAIN_VPUG1>;
+		power-domains = <&pgc_vpumix>;
	};

	pgc_vpu_g2: power-domain@8 {
		#power-domain-cells = <0>;
		reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
+		power-domains = <&pgc_vpumix>;
	};

	pgc_vpu_h1: power-domain@9 {
		#power-domain-cells = <0>;
		reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
+		power-domains = <&pgc_vpumix>;
	};


I will prepare the patch to send in the next couple of days.

Regards,
Vitor Soares

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Conor Dooley @ 2024-04-05 16:13 UTC (permalink / raw)
  To: Jason-JH Lin (林睿祥)
  Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
	Houlong Wei (魏厚龙),
	devicetree@vger.kernel.org, Shawn Sung (宋孝謙),
	CK Hu (胡俊光), conor+dt@kernel.org,
	robh@kernel.org, linux-arm-kernel@lists.infradead.org,
	krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
	jassisinghbrar@gmail.com, angelogioacchino.delregno@collabora.com
In-Reply-To: <e6a30feb1e4bb41c90df5e0272385d0f47a7dcab.camel@mediatek.com>


[-- Attachment #1.1: Type: text/plain, Size: 4393 bytes --]

On Fri, Apr 05, 2024 at 02:33:14PM +0000, Jason-JH Lin (林睿祥) wrote:
> On Thu, 2024-04-04 at 15:52 +0100, Conor Dooley wrote:
> > On Thu, Apr 04, 2024 at 04:31:06AM +0000, Jason-JH Lin (林睿祥) wrote:
> > > Hi Conor,
> > > 
> > > Thanks for the reviews.
> > > 
> > > On Wed, 2024-04-03 at 16:46 +0100, Conor Dooley wrote:
> > > > On Wed, Apr 03, 2024 at 06:25:54PM +0800, Shawn Sung wrote:
> > > > > From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> > > > > 
> > > > > Add mboxes to define a GCE loopping thread as a secure irq
> > > > > handler.
> > > > > This property is only required if CMDQ secure driver is
> > > > > supported.
> > > > > 
> > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > > > ---
> > > > >  .../bindings/mailbox/mediatek,gce-mailbox.yaml         | 10
> > > > > ++++++++++
> > > > >  1 file changed, 10 insertions(+)
> > > > > 
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > index cef9d76013985..c0d80cc770899 100644
> > > > > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > mailbox.yaml
> > > > > @@ -49,6 +49,16 @@ properties:
> > > > >      items:
> > > > >        - const: gce
> > > > >  
> > > > > +  mediatek,gce-events:
> > > > > +    description:
> > > > > +      The event id which is mapping to the specific hardware
> > > > > event
> > > > > signal
> > > > > +      to gce. The event id is defined in the gce header
> > > > > +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > > > 
> > > > Missing any info here about when this should be used, hint - you
> > > > have
> > > > it
> > > > in the commit message.
> > > > 
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-arrayi
> > > > 
> > > > Why is the ID used by the CMDQ service not fixed for each SoC?
> > > > 
> > > 
> > > I forgot to sync with Shawn about this:
> > > https://lore.kernel.org/all/20240124011459.12204-1-jason-
> > > jh.lin@mediatek.com
> > > 
> > > I'll fix it at the next version.
> > 
> > When I say "fixed" I don't mean "this is wrong, please fix it", I
> > mean
> > "why is the value not static for a particular SoC". This needs to be
> > explained in the patch (and the description for the event here needs
> > to
> > explain what the gce-mailbox is reserving an event for).
> > 
> Oh, I see. Thanks for noticing me.
> 
> We do want to reserve a static event ID for gce-mailbox to different
> SoCs. There are 2 mainly reasons to why we set it in DTS:
> 1. There are 1024 events IDs for GCE to use to execute instructions in
> the specific event happened. These events could be signaled by HW or SW
> and their value would be different in different SoC because of HW event
> IDs distribution range from 0 to 1023.
> If we set a static event ID: 855 for mt8188, it might be conflict the
> event ID original set in mt8195.

That's not a problem, we have compatibles for this purpose.

> 2. If we defined the event ID in DTS, we might know how many SW or HW
> event IDs are used.
> If someone wants to use a new event ID for a new feature, they could
> find out the used event IDs in DTS easily and avoid the event ID
> conflicting.

Are the event IDs not documented in the reference manual for the SoC in
question? Or in documentation for the secure world for these devices? A
DTS should not be the authoritive source for this information for
developers.

Additionally, the driver could very easily detect if someone does happen
to put in the reserved ID. That could be generically useful (IOW, check
all of them for re-use) if the ID are to not allowed to be shared.

> The reason why we define a event ID is we want to get a SW signal from
> secure world. We design a GCE looping thread in gce-mailbox driver to
> wait for the GCE execute done event for each cmdq secure packets from
> secure world.

This sort of information needs to be in the commit message, but I don't
think this property is needed at all since it seems to be something
detectable from the compatible.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2 2/2] arm64: dts: rockchip: add Protonic MECSBC device-tree
From: Andrew Lunn @ 2024-04-05 16:12 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
	devicetree, linux-arm-kernel, linux-rockchip, linux-kernel,
	David Jander
In-Reply-To: <20240405-protonic-mecsbc-v2-2-0a6fedc78b9f@pengutronix.de>

> +&gmac1 {
> +	assigned-clocks = <&cru SCLK_GMAC1_RX_TX>, <&cru SCLK_GMAC1>;
> +	assigned-clock-parents = <&cru SCLK_GMAC1_RGMII_SPEED>, <&cru CLK_MAC1_2TOP>;
> +	phy-handle = <&rgmii_phy1>;
> +	phy-mode = "rgmii-id";
> +	clock_in_out = "output";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&gmac1m1_miim
> +		     &gmac1m1_tx_bus2
> +		     &gmac1m1_rx_bus2
> +		     &gmac1m1_rgmii_clk
> +		     &gmac1m1_clkinout
> +		     &gmac1m1_rgmii_bus>;
> +	status = "okay";
> +};

Thank for changing it.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 3/4] usb: dwc3: exynos: Use devm_regulator_bulk_get_enable() helper function
From: Christophe JAILLET @ 2024-04-05 16:12 UTC (permalink / raw)
  To: Anand Moon
  Cc: Thinh Nguyen, Greg Kroah-Hartman, Krzysztof Kozlowski,
	Alim Akhtar, linux-usb, linux-arm-kernel, linux-samsung-soc,
	linux-kernel
In-Reply-To: <CANAwSgQk10=K6Z5OzvT3OUncfr6BWyx7oH2JKN5CJAnS+uO7QQ@mail.gmail.com>

Le 05/04/2024 à 08:10, Anand Moon a écrit :
>   Hi Christophe, Krzysztof,
> 
> On Mon, 4 Mar 2024 at 17:16, Anand Moon <linux.amoon@gmail.com> wrote:
>>
>> Hi Christophe,
>>
>> On Sun, 3 Mar 2024 at 00:07, Christophe JAILLET
>> <christophe.jaillet@wanadoo.fr> wrote:
>>>
>>> Le 02/03/2024 à 17:48, Anand Moon a écrit :
>>>> Hi Christophe,
>>>>
>>>> On Sat, 2 Mar 2024 at 21:20, Christophe JAILLET
>>>> <christophe.jaillet@wanadoo.fr> wrote:
>>>>>
>>>>> Le 01/03/2024 à 20:38, Anand Moon a écrit :
>>>>>> Use devm_regulator_bulk_get_enable() instead of open coded
>>>>>> 'devm_regulator_get(), regulator_enable(), regulator_disable().
>>>>>>
>>>>>> Signed-off-by: Anand Moon <linux.amoon@gmail.com>
>>>>>> ---
>>>>>>     drivers/usb/dwc3/dwc3-exynos.c | 49 +++-------------------------------
>>>>>>     1 file changed, 4 insertions(+), 45 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>>>>>> index 5d365ca51771..7c77f3c69825 100644
>>>>>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>>>>>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>>>>>> @@ -32,9 +32,6 @@ struct dwc3_exynos {
>>>>>>         struct clk              *clks[DWC3_EXYNOS_MAX_CLOCKS];
>>>>>>         int                     num_clks;
>>>>>>         int                     suspend_clk_idx;
>>>>>> -
>>>>>> -     struct regulator        *vdd33;
>>>>>> -     struct regulator        *vdd10;
>>>>>>     };
>>>>>>
>>>>>>     static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>> @@ -44,6 +41,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>>         struct device_node      *node = dev->of_node;
>>>>>>         const struct dwc3_exynos_driverdata *driver_data;
>>>>>>         int                     i, ret;
>>>>>> +     static const char * const regulators[] = { "vdd33", "vdd10" };
>>>>>>
>>>>>>         exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
>>>>>>         if (!exynos)
>>>>>> @@ -78,27 +76,9 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>>         if (exynos->suspend_clk_idx >= 0)
>>>>>>                 clk_prepare_enable(exynos->clks[exynos->suspend_clk_idx]);
>>>>>>
>>>>>> -     exynos->vdd33 = devm_regulator_get(dev, "vdd33");
>>>>>> -     if (IS_ERR(exynos->vdd33)) {
>>>>>> -             ret = PTR_ERR(exynos->vdd33);
>>>>>> -             goto vdd33_err;
>>>>>> -     }
>>>>>> -     ret = regulator_enable(exynos->vdd33);
>>>>>> -     if (ret) {
>>>>>> -             dev_err(dev, "Failed to enable VDD33 supply\n");
>>>>>> -             goto vdd33_err;
>>>>>> -     }
>>>>>> -
>>>>>> -     exynos->vdd10 = devm_regulator_get(dev, "vdd10");
>>>>>> -     if (IS_ERR(exynos->vdd10)) {
>>>>>> -             ret = PTR_ERR(exynos->vdd10);
>>>>>> -             goto vdd10_err;
>>>>>> -     }
>>>>>> -     ret = regulator_enable(exynos->vdd10);
>>>>>> -     if (ret) {
>>>>>> -             dev_err(dev, "Failed to enable VDD10 supply\n");
>>>>>> -             goto vdd10_err;
>>>>>> -     }
>>>>>> +     ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators), regulators);
>>>>>> +     if (ret)
>>>>>> +             return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>>>>>>
>>>>>>         if (node) {
>>>>>>                 ret = of_platform_populate(node, NULL, NULL, dev);
>>>>>> @@ -115,10 +95,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>>>>>         return 0;
>>>>>>
>>>>>>     populate_err:
>>>>>> -     regulator_disable(exynos->vdd10);
>>>>>> -vdd10_err:
>>>>>> -     regulator_disable(exynos->vdd33);
>>>>>> -vdd33_err:
>>>>>>         for (i = exynos->num_clks - 1; i >= 0; i--)
>>>>>>                 clk_disable_unprepare(exynos->clks[i]);
>>>>>>
>>>>>> @@ -140,9 +116,6 @@ static void dwc3_exynos_remove(struct platform_device *pdev)
>>>>>>
>>>>>>         if (exynos->suspend_clk_idx >= 0)
>>>>>>                 clk_disable_unprepare(exynos->clks[exynos->suspend_clk_idx]);
>>>>>> -
>>>>>> -     regulator_disable(exynos->vdd33);
>>>>>> -     regulator_disable(exynos->vdd10);
>>>>>>     }
>>>>>>
>>>>>>     static const struct dwc3_exynos_driverdata exynos5250_drvdata = {
>>>>>> @@ -196,9 +169,6 @@ static int dwc3_exynos_suspend(struct device *dev)
>>>>>>         for (i = exynos->num_clks - 1; i >= 0; i--)
>>>>>>                 clk_disable_unprepare(exynos->clks[i]);
>>>>>>
>>>>>> -     regulator_disable(exynos->vdd33);
>>>>>> -     regulator_disable(exynos->vdd10);
>>>>>
>>>>> Hi,
>>>>>
>>>>> Same here, I don't think that removing regulator_[en|dis]able from the
>>>>> suspend and resume function is correct.
>>>>>
>>>>> The goal is to stop some hardware when the system is suspended, in order
>>>>> to save some power.
>>>> Ok,
>>>>>
>>>>> Why did you removed it?
>>>>
>>>> As per the description of the function  devm_regulator_bulk_get_enable
>>>>
>>>> * This helper function allows drivers to get several regulator
>>>>    * consumers in one operation with management, the regulators will
>>>>    * automatically be freed when the device is unbound.  If any of the
>>>>    * regulators cannot be acquired then any regulators that were
>>>>    * allocated will be freed before returning to the caller.
>>>
>>> The code in suspend/resume is not about freeing some resources. It is
>>> about enabling/disabling some hardware to save some power.
>>>
>>> Think to the probe/remove functions as the software in the kernel that
>>> knows how to handle some hardawre, and the suspend/resume as the on/off
>>> button to power-on and off the electrical chips.
>>>
>>> When the system is suspended, the software is still around. But some
>>> hardware can be set in a low consumption mode to save some power.
>>>
>>> IMHO, part of the code you removed changed this behaviour and increase
>>> the power consumption when the system is suspended.
>>>
>>
>> You are correct, I have changed the regulator API from
>> devm_regulator_get_enable to devm_regulator_bulk_get_enable
>> which changes this behavior.
>> I will fix it in the next version.
>>
>>> CJ
> 
> I could not find any example in the kernel to support
> devm_regulator_bulk_disable
> but here is my modified file.
> 
> If you have any suggestions for this plz let me know.

I don't think that your approach is correct, and I don't think that the 
proposed patch does what you expect it to do.

Calling a devm_ function in suspend/resume functions looks really 
strange to me and is likely broken.

Especially here, devm_regulator_bulk_get_enable() in the resume function 
allocates some memory that is not freed in 
devm_regulator_bulk_disable(), because the API is not designed to work 
like that. So this could generate a kind of memory leak.


*I think that the code is good enough as-is*, but if you really want to 
change something, maybe:
    - devm_regulator_get()+regulator_enable() in the probe could be 
changed to devm_regulator_get_enable()
    - the resume/suspend function should be left as-is with 
regulator_disable()/regulator_ensable()
    - remove regulator_disable() from the error handling path of the 
probe and from the remove function.

I *think* it would work.

CJ


> -----8<----------8<----------
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 6d07592ad022..2f808cb9a006 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -34,6 +34,8 @@ struct dwc3_exynos {
>          int                     suspend_clk_idx;
>   };
> 
> +static const char * const regulators[] = { "vdd33", "vdd10" };
> +
>   static int dwc3_exynos_probe(struct platform_device *pdev)
>   {
>          struct dwc3_exynos      *exynos;
> @@ -41,7 +43,6 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>          struct device_node      *node = dev->of_node;
>          const struct dwc3_exynos_driverdata *driver_data;
>          int                     i, ret;
> -       static const char * const regulators[] = { "vdd33", "vdd10" };
> 
>          exynos = devm_kzalloc(dev, sizeof(*exynos), GFP_KERNEL);
>          if (!exynos)
> @@ -166,6 +167,8 @@ static int __maybe_unused
> dwc3_exynos_suspend(struct device *dev)
>          struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>          int i;
> 
> +       devm_regulator_bulk_disable(dev);
> +
>          for (i = exynos->num_clks - 1; i >= 0; i--)
>                  clk_disable_unprepare(exynos->clks[i]);
> 
> @@ -177,6 +180,11 @@ static int __maybe_unused
> dwc3_exynos_resume(struct device *dev)
>          struct dwc3_exynos *exynos = dev_get_drvdata(dev);
>          int i, ret;
> 
> +       ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(regulators),
> +                                               regulators);
> +       if (ret)
> +               dev_err(dev, "Failed to enable regulators\n");
> +
>          for (i = 0; i < exynos->num_clks; i++) {
>                  ret = clk_prepare_enable(exynos->clks[i]);
>                  if (ret) {
> 
> To support these changes we need to export the
> devm_regulator_bulk_disable function.
> -----8<----------8<----------
> diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
> index 90bb0d178885..97eed739f929 100644
> --- a/drivers/regulator/devres.c
> +++ b/drivers/regulator/devres.c
> @@ -318,7 +318,7 @@ void devm_regulator_bulk_put(struct
> regulator_bulk_data *consumers)
>   }
>   EXPORT_SYMBOL_GPL(devm_regulator_bulk_put);
> 
> -static void devm_regulator_bulk_disable(void *res)
> +void devm_regulator_bulk_disable(void *res)
>   {
>          struct regulator_bulk_devres *devres = res;
>          int i;
> @@ -326,6 +326,7 @@ static void devm_regulator_bulk_disable(void *res)
>          for (i = 0; i < devres->num_consumers; i++)
>                  regulator_disable(devres->consumers[i].consumer);
>   }
> +EXPORT_SYMBOL_GPL(devm_regulator_bulk_disable);
> 
>   /**
>    * devm_regulator_bulk_get_enable - managed get'n enable multiple regulators
> diff --git a/include/linux/regulator/consumer.h
> b/include/linux/regulator/consumer.h
> index 4660582a3302..ce7d28306b17 100644
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
> @@ -214,6 +214,7 @@ int __must_check regulator_bulk_enable(int num_consumers,
>                                         struct regulator_bulk_data *consumers);
>   int devm_regulator_bulk_get_enable(struct device *dev, int num_consumers,
>                                     const char * const *id);
> +void devm_regulator_bulk_disable(void *res);
>   int regulator_bulk_disable(int num_consumers,
>                             struct regulator_bulk_data *consumers);
>   int regulator_bulk_force_disable(int num_consumers,
> --------------------------------------------------------------------------
> 
> Thanks
> -Anand
> 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [syzbot] [mm?] BUG: unable to handle kernel paging request in copy_from_kernel_nofault (2)
From: Alexei Starovoitov @ 2024-04-05 16:12 UTC (permalink / raw)
  To: Russell King (Oracle), Puranjay Mohan
  Cc: Mark Rutland, Andrew Morton, linux-arm-kernel, syzbot, LKML,
	linux-mm, syzkaller-bugs, bpf
In-Reply-To: <Zg/iGQCDKa9bllyI@shell.armlinux.org.uk>

On Fri, Apr 5, 2024 at 4:36 AM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Fri, Apr 05, 2024 at 12:02:36PM +0100, Mark Rutland wrote:
> > On Thu, Apr 04, 2024 at 03:57:04PM -0700, Alexei Starovoitov wrote:
> > > On Wed, Apr 3, 2024 at 6:56 PM Andrew Morton <akpm@linux-foundationorg> wrote:
> > > >
> > > > On Mon, 01 Apr 2024 22:19:25 -0700 syzbot <syzbot+186522670e6722692d86@syzkaller.appspotmail.com> wrote:
> > > >
> > > > > Hello,
> > > >
> > > > Thanks.  Cc: bpf@vger.kernel.org
> > >
> > > I suspect the issue is not on bpf side.
> > > Looks like the bug is somewhere in arm32 bits.
> > > copy_from_kernel_nofault() is called from lots of places.
> > > bpf is just one user that is easy for syzbot to fuzz.
> > > Interestingly arm defines copy_from_kernel_nofault_allowed()
> > > that should have filtered out user addresses.
> > > In this case ffffffe9 is probably a kernel address?
> >
> > It's at the end of the kernel range, and it's ERR_PTR(-EINVAL).
> >
> > 0xffffffe9 is -0x16, which is -22, which is -EINVAL.
> >
> > > But the kernel is doing a write?
> > > Which makes no sense, since copy_from_kernel_nofault is probe reading.
> >
> > It makes perfect sense; the read from 'src' happened, then the kernel tries to
> > write the result to 'dst', and that aligns with the disassembly in the report
> > below, which I beleive is:
> >
> >      8: e4942000        ldr     r2, [r4], #0  <-- Read of 'src', fault fixup is elsewhere
> >      c: e3530000        cmp     r3, #0
> >   * 10: e5852000        str     r2, [r5]      <-- Write to 'dst'
> >
> > As above, it looks like 'dst' is ERR_PTR(-EINVAL).
> >
> > Are you certain that BPF is passing a sane value for 'dst'? Where does that
> > come from in the first place?
>
> It looks to me like it gets passed in from the BPF program, and the
> "type" for the argument is set to ARG_PTR_TO_UNINIT_MEM. What that
> means for validation purposes, I've no idea, I'm not a BPF hacker.
>
> Obviously, if BPF is allowing copy_from_kernel_nofault() to be passed
> an arbitary destination address, that would be a huge security hole.

If that's the case that's indeed a giant security hole,
but I doubt it. We would be crashing other archs as well.
I cannot really tell whether arm32 JIT is on.
If it is, it's likely a bug there.
Puranjay,
could you please take a look.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v1 4/4] arm64: dts: freescale: imx8mm-verdin-dahlia: support sleep-moci
From: Stefan Eichenberger @ 2024-04-05 16:07 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini
  Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
	Stefan Eichenberger
In-Reply-To: <20240405160720.5977-1-eichest@gmail.com>

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Previously, we had the sleep-moci pin set to always on. However, the
Dahlia carrier board supports disabling the sleep-moci when the system
is suspended to power down peripherals that support it. This reduces
overall power consumption. This commit adds support for this feature by
disabling the reg_force_sleep_moci regulator and adding two new
regulators for the USB hub and PCIe that can be turned off when the
system is suspended.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 .../dts/freescale/imx8mm-verdin-dahlia.dtsi   | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
index b64dac4f29c2..393fc9e20423 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
@@ -32,6 +32,25 @@ simple-audio-card,cpu {
 			sound-dai = <&sai2>;
 		};
 	};
+
+	reg_usb_hub: regulator-usb-hub {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+		gpio = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+		regulator-boot-on;
+		regulator-name = "HUB_PWR_EN";
+	};
+
+	reg_pcie: regulator-pcie {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+		gpio = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+		regulator-boot-on;
+		regulator-name = "PCIE_1_PWR_EN";
+		startup-delay-us = <100000>;
+	};
 };
 
 /* Verdin SPI_1 */
@@ -98,6 +117,7 @@ wm8904_1a: audio-codec@1a {
 
 /* Verdin PCIE_1 */
 &pcie0 {
+	vpcie-supply = <&reg_pcie>;
 	status = "okay";
 };
 
@@ -120,6 +140,11 @@ &pwm3 {
 	status = "okay";
 };
 
+/* We support turning off sleep moci on Dahlia */
+&reg_force_sleep_moci {
+	status = "disabled";
+};
+
 /* Verdin I2S_1 */
 &sai2 {
 	status = "okay";
@@ -148,8 +173,16 @@ &usbotg1 {
 
 /* Verdin USB_2 */
 &usbotg2 {
+	#address-cells = <1>;
+	#size-cells = <0>;
 	disable-over-current;
 	status = "okay";
+
+	usb-hub@1 {
+		compatible = "usb424,2744";
+		reg = <1>;
+		vdd-supply = <&reg_usb_hub>;
+	};
 };
 
 /* Verdin SD_1 */
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v1 1/4] arm64: dts: freescale: imx8mp-verdin: replace sleep-moci hog with regulator
From: Stefan Eichenberger @ 2024-04-05 16:07 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini
  Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
	Stefan Eichenberger
In-Reply-To: <20240405160720.5977-1-eichest@gmail.com>

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

The Verdin family has a signal called sleep-moci which can be used to
turn off peripherals on the carrier board when the SoM goes into
suspend. So far we have hogged this signal, which means the peripherals
are always on and it is not possible to add peripherals that depend on
the sleep-moci to be on. With this change, we replace the hog with a
regulator so that peripherals can add their own regulators that use the
same gpio. Carrier boards that allow peripherals to be powered off in
suspend can disable this regulator and implement their own regulator to
control the sleep-moci.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 .../dts/freescale/imx8mp-verdin-dahlia.dtsi   |  5 ++++
 .../boot/dts/freescale/imx8mp-verdin-dev.dtsi |  5 ++++
 .../dts/freescale/imx8mp-verdin-yavia.dtsi    |  5 ++++
 .../boot/dts/freescale/imx8mp-verdin.dtsi     | 26 ++++++++++++-------
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
index 7e9e4b13b5c5..e68e0e6f21e9 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi
@@ -70,6 +70,11 @@ &flexspi {
 	status = "okay";
 };
 
+&gpio4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 /* Current measurement into module VCC */
 &hwmon {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi
index a509b2b7fa85..1a2520d4d6cf 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi
@@ -93,6 +93,11 @@ &flexspi {
 	status = "okay";
 };
 
+&gpio4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 &gpio_expander_21 {
 	status = "okay";
 	vcc-supply = <&reg_1p8v>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-yavia.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-yavia.dtsi
index db1722f0d80e..27160024d5b5 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin-yavia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-yavia.dtsi
@@ -100,6 +100,11 @@ &flexcan1 {
 	status = "okay";
 };
 
+&gpio4 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 &hwmon_temp {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
index faa17cbbe2fd..e523762947aa 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
@@ -116,6 +116,22 @@ reg_module_eth1phy: regulator-module-eth1phy {
 		vin-supply = <&reg_vdd_3v3>;
 	};
 
+	/*
+	 * By default we enable CTRL_SLEEP_MOCI#, this is required to have
+	 * peripherals on the carrier board powered.
+	 * If more granularity or power saving is required this can be disabled
+	 * in the carrier board device tree files.
+	 */
+	reg_force_sleep_moci: regulator-force-sleep-moci {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+		gpio = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-name = "CTRL_SLEEP_MOCI#";
+	};
+
 	reg_usb1_vbus: regulator-usb1-vbus {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -439,16 +455,6 @@ &gpio4 {
 			  "SODIMM_256",
 			  "SODIMM_48",
 			  "SODIMM_44";
-
-	ctrl-sleep-moci-hog {
-		gpio-hog;
-		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
-		gpios = <29 GPIO_ACTIVE_HIGH>;
-		line-name = "CTRL_SLEEP_MOCI#";
-		output-high;
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
-	};
 };
 
 /* On-module I2C */
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v1 3/4] arm64: dts: freescale: imx8mm-verdin: replace sleep-moci hog with regulator
From: Stefan Eichenberger @ 2024-04-05 16:07 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini
  Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
	Stefan Eichenberger
In-Reply-To: <20240405160720.5977-1-eichest@gmail.com>

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

The Verdin family has a signal called sleep-moci which can be used to
turn off peripherals on the carrier board when the SoM goes into
suspend. So far we have hogged this signal, which means the peripherals
are always on and it is not possible to add peripherals that depend on
the sleep-moci to be on. With this change, we replace the hog with a
regulator so that peripherals can add their own regulators that use the
same gpio. Carrier boards that allow peripherals to be powered off in
suspend can disable this regulator and implement their own regulator to
control the sleep-moci.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 .../dts/freescale/imx8mm-verdin-dahlia.dtsi   |  5 ++++
 .../boot/dts/freescale/imx8mm-verdin-dev.dtsi |  5 ++++
 .../dts/freescale/imx8mm-verdin-yavia.dtsi    |  5 ++++
 .../boot/dts/freescale/imx8mm-verdin.dtsi     | 26 ++++++++++++-------
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
index 1cff0b829357..b64dac4f29c2 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dahlia.dtsi
@@ -58,6 +58,11 @@ &flexspi {
 	status = "okay";
 };
 
+&gpio5 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 /* Current measurement into module VCC */
 &hwmon {
 	status = "okay";
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
index 3c4b8ca125e3..95b7c9a03a23 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-dev.dtsi
@@ -78,6 +78,11 @@ &i2c3 {
 	status = "okay";
 };
 
+&gpio5 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 &gpio_expander_21 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin-yavia.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin-yavia.dtsi
index 1e28c78e381f..763f069e8405 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin-yavia.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin-yavia.dtsi
@@ -81,6 +81,11 @@ &gpio3 {
 	pinctrl-0 = <&pinctrl_gpios_ext_yavia>;
 };
 
+&gpio5 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
+};
+
 &hwmon_temp {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
index 6f0811587142..4768b05fd765 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
@@ -110,6 +110,22 @@ reg_ethphy: regulator-ethphy {
 		startup-delay-us = <200000>;
 	};
 
+	/*
+	 * By default we enable CTRL_SLEEP_MOCI#, this is required to have
+	 * peripherals on the carrier board powered.
+	 * If more granularity or power saving is required this can be disabled
+	 * in the carrier board device tree files.
+	 */
+	reg_force_sleep_moci: regulator-force-sleep-moci {
+		compatible = "regulator-fixed";
+		enable-active-high;
+		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
+		gpio = <&gpio5 1 GPIO_ACTIVE_HIGH>;
+		regulator-always-on;
+		regulator-boot-on;
+		regulator-name = "CTRL_SLEEP_MOCI#";
+	};
+
 	reg_usb_otg1_vbus: regulator-usb-otg1 {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -333,16 +349,6 @@ &gpio5 {
 			  "SODIMM_212",
 			  "SODIMM_151",
 			  "SODIMM_153";
-
-	ctrl-sleep-moci-hog {
-		gpio-hog;
-		/* Verdin CTRL_SLEEP_MOCI# (SODIMM 256) */
-		gpios = <1 GPIO_ACTIVE_HIGH>;
-		line-name = "CTRL_SLEEP_MOCI#";
-		output-high;
-		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_ctrl_sleep_moci>;
-	};
 };
 
 /* On-module I2C */
-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v1 0/4] arm64: dts: freescale: verdin-imx8mm/imx8mp: add sleep-moci support
From: Stefan Eichenberger @ 2024-04-05 16:07 UTC (permalink / raw)
  To: robh, krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, francesco.dolcini
  Cc: devicetree, imx, linux-arm-kernel, linux-kernel,
	Stefan Eichenberger

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

This patch series adds support for sleep-moci to the Verdin iMX8MM and
iMX8MP in combination with the Dahlia carrier board. sleep-moci is a
GPIO that allows the system on module to turn off regulators that are
not needed in suspend mode on the carrier board.

Stefan Eichenberger (4):
  arm64: dts: freescale: imx8mp-verdin: replace sleep-moci hog with
    regulator
  arm64: dts: freescale: imx8mp-verdin-dahlia: support sleep-moci
  arm64: dts: freescale: imx8mm-verdin: replace sleep-moci hog with
    regulator
  arm64: dts: freescale: imx8mm-verdin-dahlia: support sleep-moci

 .../dts/freescale/imx8mm-verdin-dahlia.dtsi   | 39 +++++++++++++++
 .../boot/dts/freescale/imx8mm-verdin-dev.dtsi |  5 ++
 .../dts/freescale/imx8mm-verdin-yavia.dtsi    |  5 ++
 .../boot/dts/freescale/imx8mm-verdin.dtsi     | 26 ++++++----
 .../dts/freescale/imx8mp-verdin-dahlia.dtsi   | 50 +++++++++++++++++++
 .../boot/dts/freescale/imx8mp-verdin-dev.dtsi |  5 ++
 .../dts/freescale/imx8mp-verdin-yavia.dtsi    |  5 ++
 .../boot/dts/freescale/imx8mp-verdin.dtsi     | 26 ++++++----
 8 files changed, 141 insertions(+), 20 deletions(-)

-- 
2.40.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 6/6] dts: qcom: sa8775p-ride: remove tx-sched-sp property
From: Krzysztof Kozlowski @ 2024-04-05 15:52 UTC (permalink / raw)
  To: Flavio Suligoi, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Bjorn Andersson, Konrad Dybcio, Giuseppe Cavallaro
  Cc: netdev, linux-stm32, linux-arm-kernel, devicetree, imx,
	linux-arm-msm, linux-kernel
In-Reply-To: <20240405152800.638461-7-f.suligoi@asem.it>

On 05/04/2024 17:28, Flavio Suligoi wrote:
> The property "tx-sched-sp" no longer exists, as it was removed from the
> file:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> 
> by the commit:
> 
> commit aed6864035b1 ("net: stmmac: platform: Delete a redundant condition
> branch")
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>

Fixes in commit msg needed and patch prefix got mangled: missing arm64.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 5/6] arm64: dts: qcom: sa8540p-ride: remove tx-sched-sp property
From: Krzysztof Kozlowski @ 2024-04-05 15:51 UTC (permalink / raw)
  To: Flavio Suligoi, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Bjorn Andersson, Konrad Dybcio, Giuseppe Cavallaro
  Cc: netdev, linux-stm32, linux-arm-kernel, devicetree, imx,
	linux-arm-msm, linux-kernel
In-Reply-To: <20240405152800.638461-6-f.suligoi@asem.it>

On 05/04/2024 17:27, Flavio Suligoi wrote:
> The property "tx-sched-sp" no longer exists, as it was removed from the
> file:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> 
> by the commit:
> 
> commit aed6864035b1 ("net: stmmac: platform: Delete a redundant condition
> branch")

Same problem with commit. BTW, your commit msg does not say what happens
if this property is removed. Instead it could be "Strict priority is by
default in Linux driver and tx-sched-sp was removed in commit sha ("foo
bar")".


With fixed in commit msg.

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/6] dt-bindings: net: snps,dwmac: remove tx-sched-sp property
From: Krzysztof Kozlowski @ 2024-04-05 15:50 UTC (permalink / raw)
  To: Flavio Suligoi, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Bjorn Andersson, Konrad Dybcio, Giuseppe Cavallaro
  Cc: netdev, linux-stm32, linux-arm-kernel, devicetree, imx,
	linux-arm-msm, linux-kernel
In-Reply-To: <20240405152800.638461-2-f.suligoi@asem.it>

On 05/04/2024 17:27, Flavio Suligoi wrote:
> The property "tx-sched-sp" no longer exists, as it was removed from the
> file:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> 
> by the commit:
> 
> commit aed6864035b1 ("net: stmmac: platform: Delete a redundant condition
> branch")
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml        | 14 --------------
>  1 file changed, 14 deletions(-)

One more thought though:
1. Missing net-next patch annotation,
2. Please split DTS from net. DTS goes via separate trees.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/6] dt-bindings: net: snps,dwmac: remove tx-sched-sp property
From: Krzysztof Kozlowski @ 2024-04-05 15:49 UTC (permalink / raw)
  To: Flavio Suligoi, Alexandre Torgue, Jose Abreu, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Bjorn Andersson, Konrad Dybcio, Giuseppe Cavallaro
  Cc: netdev, linux-stm32, linux-arm-kernel, devicetree, imx,
	linux-arm-msm, linux-kernel
In-Reply-To: <20240405152800.638461-2-f.suligoi@asem.it>

On 05/04/2024 17:27, Flavio Suligoi wrote:
> The property "tx-sched-sp" no longer exists, as it was removed from the
> file:
> 
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> 
> by the commit:

Keep syntax as asked by submitting patches, so "by the commit sha ("foo
bar").

> 
> commit aed6864035b1 ("net: stmmac: platform: Delete a redundant condition
> branch")
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml        | 14 --------------
>  1 file changed, 14 deletions(-)

This means by default we have tx-sched-sp... I guess it is fine,
assuming there are no other users (projects) of this binding property.

Acked-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox