* Re: [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support
From: Linus Walleij @ 2026-05-13 7:40 UTC (permalink / raw)
To: Yu-Chun Lin
Cc: brgl, robh, krzk+dt, conor+dt, afaerber, wbg,
mathieu.dubois-briand, mwalle, lars, Michael.Hennerich, jic23,
nuno.sa, andy, dlechner, tychang, linux-gpio, devicetree,
linux-kernel, linux-arm-kernel, linux-realtek-soc, linux-iio,
cy.huang, stanley_chang, james.tai, Linus Walleij
In-Reply-To: <20260512033317.1602537-4-eleanor.lin@realtek.com>
Hi Yu-Chun,
thanks for your patch! I really like the direction this is taking.
On Tue, May 12, 2026 at 5:33 AM Yu-Chun Lin <eleanor.lin@realtek.com> wrote:
> Extend the reg_mask_xlate callback with an operation type parameter
> (gpio_regmap_operation) to allow drivers to return different
> register/mask combinations for different GPIO operations.
>
> Also add write-enable mechanism for hardware that requires setting a
> write-enable bit before modifying GPIO control registers.
>
> Consequently, update all existing drivers utilizing the gpio-regmap
> framework (across drivers/gpio, drivers/iio, and drivers/pinctrl)
> to accommodate the new reg_mask_xlate function signature.
>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
linusw@kernel.org these days.
> Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
The rest of the comments I had Andy, Jonathan and Sashiko has
already pointed out, just hash through it and this will look really
nice in the end.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables
From: Arnaud POULIQUEN @ 2026-05-13 7:37 UTC (permalink / raw)
To: Daniel Baluta, tanmay.shah, Ben Levinsky, Bjorn Andersson,
Mathieu Poirier, linux-remoteproc
Cc: Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Geert Uytterhoeven, Magnus Damm, Patrice Chotard, Maxime Coquelin,
Alexandre Torgue, imx, linux-arm-kernel, linux-kernel,
linux-renesas-soc, linux-stm32
In-Reply-To: <3b1c5579-a9de-4955-ad3c-df3f8c1860f4@oss.nxp.com>
On 5/13/26 08:30, Daniel Baluta wrote:
> On 5/12/26 17:53, Shah, Tanmay wrote:
>>
>>
>> On 5/12/2026 2:55 AM, Daniel Baluta wrote:
>>> On 5/12/26 00:18, Ben Levinsky wrote:
>>>> [You don't often get email from ben.levinsky@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>>
>>>> Add a small helper around rproc_elf_load_rsc_table() for remoteproc
>>>> drivers that treat a missing ELF resource table as optional. The helper
>>>> returns success on -EINVAL and propagates other failures unchanged.
>>>>
>>>> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
>>>> ---
>>>> drivers/remoteproc/remoteproc_internal.h | 12 ++++++++++++
>>>> 1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
>>>> index 3724a47a9748..dff87e468837 100644
>>>> --- a/drivers/remoteproc/remoteproc_internal.h
>>>> +++ b/drivers/remoteproc/remoteproc_internal.h
>>>> @@ -146,6 +146,18 @@ static inline int rproc_mem_entry_iounmap(struct rproc *rproc,
>>>> return 0;
>>>> }
>>>>
>>>> +static inline int rproc_elf_load_rsc_table_optional(struct rproc *rproc,
>>>> + const struct firmware *fw)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + ret = rproc_elf_load_rsc_table(rproc, fw);
>>>> + if (ret == -EINVAL)
>>>> + dev_dbg(&rproc->dev, "no resource table found\n");
>>>
>>> You are changing loglevel here. Initial drivers use dev_info or dev_warn. At least I'm used
>>> with seeing this messages in the logs.
>>>
>>> So, what do you think on adding at least dev_info to this instead of dev_dbg?
>>>
>>
>> Actually can we leave that choice to the platform driver ? There are
>> many use cases where the remoteproc subsystem is used to load and start
>> the remote core and the firmware doesn't have the resource table. We
>> don't want to make info level log for such use cases, as the resource
>> table is not expected in the first place there.
>
> Agree, this is the best way to go.
>
>
LGTM
If you keep the rproc_elf_load_rsc_table_optional() helper, I would
suggest inverting the logic for dev_dbg(). Regarding the discussion, it
seems more logical to print a message when a resource table is found.
An add-on could be to also print the address and size found.
Thanks,
Arnaud
^ permalink raw reply
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Hans Zhang @ 2026-05-13 7:34 UTC (permalink / raw)
To: Pali Rohár
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <20260513072008.vol4htgbzquly2rb@pali>
On 5/13/26 15:20, Pali Rohár wrote:
> On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
>>
>>
>> On 5/13/26 05:25, Pali Rohár wrote:
>>> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>>>> The Aardvark PCIe controller driver waits for the link to come up but
>>>> does not implement the mandatory 100 ms delay after link training
>>>> completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>>>
>>>> The driver already maintains a 'link_gen' field that holds the negotiated
>>>> link speed. Use it together with pcie_wait_after_link_train() to insert
>>>> the required delay immediately after confirming that the link is up.
>>>>
>>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>>> ---
>>>> drivers/pci/controller/pci-aardvark.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>>>> index e34bea1ff0ac..526351c21c49 100644
>>>> --- a/drivers/pci/controller/pci-aardvark.c
>>>> +++ b/drivers/pci/controller/pci-aardvark.c
>>>> @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
>>>> /* check if the link is up or not */
>>>> for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
>>>> - if (advk_pcie_link_up(pcie))
>>>> + if (advk_pcie_link_up(pcie)) {
>>>> + pcie_wait_after_link_train(pcie->link_gen);
>>>> return 0;
>>>> + }
>>>> usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>>>> }
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> Are you sure that this is correct to do? Have you checked the A3720
>>> Functional Specification which describes how to bring PCIe link up?
>>>
>>> A3720 PCIe controller is buggy and needs more timing hacks to make it
>>> behave. Playing with random sleeps can break its internal logic.
>>> I'm not sure if it could be safe without proper testing.
>>>
>>> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
>>
>>
>> Hi Pali,
>>
>> 1. This driver does not support A3720.
>>
>> static const struct of_device_id advk_pcie_of_match_table[] = {
>> { .compatible = "marvell,armada-3700-pcie", },
>> {},
>> };
>> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
>>
>> If you need support for A3720, please submit the corresponding patch so that
>> Bjorn and Mani can review it.
>
> 3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
> a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
>
>>
>> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
>> in the DT. This will not affect the functionality of this patch.
>
> Whole A37xx supports only GEN2. And in DT files for 37xx should be
> already there max-link-speed.
>
> Seems that in advk_pcie_of_match_table there is no GEN3 device
> specified.
>
Hi Pali,
However, I saw many GEN3 assignments and conditions in the code.
ret = of_pci_get_max_link_speed(dev->of_node);
if (ret <= 0 || ret > 3)
pcie->link_gen = 3;
else
pcie->link_gen = ret;
static void advk_pcie_train_link(struct advk_pcie *pcie)
{
struct device *dev = &pcie->pdev->dev;
u32 reg;
int ret;
/*
* Setup PCIe rev / gen compliance based on device tree property
* 'max-link-speed' which also forces maximal link speed.
*/
reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
reg &= ~PCIE_GEN_SEL_MSK;
if (pcie->link_gen == 3)
reg |= SPEED_GEN_3;
else if (pcie->link_gen == 2)
reg |= SPEED_GEN_2;
else
reg |= SPEED_GEN_1;
advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
/*
* Set maximal link speed value also into PCIe Link Control 2 register.
* Armada 3700 Functional Specification says that default value is based
* on SPEED_GEN but tests showed that default value is always 8.0 GT/s.
*/
reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
reg &= ~PCI_EXP_LNKCTL2_TLS;
if (pcie->link_gen == 3)
reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
else if (pcie->link_gen == 2)
reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
else
reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
....
If you are certain about the relevant information. Is it understandable
that we need to delete the code related to GEN3?
Best regards,
Hans
>> 3. This patch is a common delay requirement stipulated by the PCIe
>> specification. If it is greater than GEN2, then msleep(100) will be added;
>> otherwise, there will be no such delay.
>>
>> 4. For instance, we often come across the situation where some common APIs
>> are modified, and in many cases, their functionality does not require the
>> actual development board for verification. I believe that many other
>> developers and maintainers have modified different parts of the code. For
>> example, the recent submission:
>
> Switching one API to another is one thing. But changing code which looks
> to be critical, specially when it is known that hw has bugs, can cause
> breaking of existing boards.
>
>> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
>> Author: Nam Cao <namcao@linutronix.de>
>> Date: Thu Jun 26 16:47:53 2025 +0200
>>
>> PCI: aardvark: Switch to msi_create_parent_irq_domain()
>>
>> Switch to msi_create_parent_irq_domain() from
>> pci_msi_create_irq_domain()
>> which was using legacy MSI domain setup.
>>
>>
>> And many controller drivers have been modified.
>>
>>
>> Best regards,
>> Hans
>>
>>
^ permalink raw reply
* Re: [PATCH v7 07/20] KVM: arm64: Set up FGT for Partitioned PMU
From: Oliver Upton @ 2026-05-13 7:34 UTC (permalink / raw)
To: Colton Lewis
Cc: kvm, Alexandru Elisei, Paolo Bonzini, Jonathan Corbet,
Russell King, Catalin Marinas, Will Deacon, Marc Zyngier,
Oliver Upton, Mingwei Zhang, Joey Gouly, Suzuki K Poulose,
Zenghui Yu, Mark Rutland, Shuah Khan, Ganapatrao Kulkarni,
James Clark, linux-doc, linux-kernel, linux-arm-kernel, kvmarm,
linux-perf-users, linux-kselftest
In-Reply-To: <20260504211813.1804997-8-coltonlewis@google.com>
On Mon, May 04, 2026 at 09:18:00PM +0000, Colton Lewis wrote:
> +static void __compute_hdfgrtr(struct kvm_vcpu *vcpu)
> +{
> + __compute_fgt(vcpu, HDFGRTR_EL2);
> +
> + *vcpu_fgt(vcpu, HDFGRTR_EL2) |=
> + HDFGRTR_EL2_PMOVS
> + | HDFGRTR_EL2_PMCCFILTR_EL0
> + | HDFGRTR_EL2_PMEVTYPERn_EL0
> + | HDFGRTR_EL2_PMCEIDn_EL0
> + | HDFGRTR_EL2_PMMIR_EL1;
> +}
> +
I've given this feedback at least twice already...
Operators go on the preceding line in the case of line continuations.
> +
> +/**
> + * kvm_pmu_is_partitioned() - Determine if given PMU is partitioned
> + * @pmu: Pointer to arm_pmu struct
> + *
> + * Determine if given PMU is partitioned by looking at hpmn field. The
> + * PMU is partitioned if this field is less than the number of
> + * counters in the system.
> + *
> + * Return: True if the PMU is partitioned, false otherwise
> + */
> +bool kvm_pmu_is_partitioned(struct arm_pmu *pmu)
> +{
> + if (!pmu)
> + return false;
> +
> + return pmu->max_guest_counters >= 0 &&
> + pmu->max_guest_counters <= *host_data_ptr(nr_event_counters);
> +}
> +
> +/**
> + * kvm_vcpu_pmu_is_partitioned() - Determine if given VCPU has a partitioned PMU
> + * @vcpu: Pointer to kvm_vcpu struct
> + *
> + * Determine if given VCPU has a partitioned PMU by extracting that
> + * field and passing it to :c:func:`kvm_pmu_is_partitioned`
> + *
> + * Return: True if the VCPU PMU is partitioned, false otherwise
> + */
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> + return kvm_pmu_is_partitioned(vcpu->kvm->arch.arm_pmu) &&
> + false;
> +}
Ok, I'm thoroughly confused about these predicates.
Whether or not a vCPU is using a partitioned PMU is a per-VM property.
This is separate from whether or not the backing arm_pmu has a range of
available counters for the guest to use.
It is entirely possible that a VM *isn't* using the partitioned PMU
feature (i.e. backed with perf events) yet the supporting arm_pmu has a
guest counter range.
> +#if !defined(__KVM_NVHE_HYPERVISOR__)
> +bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu);
> +bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu);
> +#else
> +static inline bool kvm_vcpu_pmu_is_partitioned(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +static inline bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +#endif
> +
Don't use ifdeffery for this. Aim to have a single definition and rely
on has_vhe() to do the rest of the work.
Thanks,
Oliver
^ permalink raw reply
* Re: [PATCH v4 0/3] gpio: Add EIO GPIO support
From: Bartosz Golaszewski @ 2026-05-13 7:30 UTC (permalink / raw)
To: linux-kernel, Shubhrajyoti Datta
Cc: Bartosz Golaszewski, git, shubhrajyoti.datta, Srinivas Neeli,
Michal Simek, Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-gpio, devicetree,
linux-arm-kernel
In-Reply-To: <20260512060917.2096456-1-shubhrajyoti.datta@amd.com>
On Tue, 12 May 2026 11:38:46 +0530, Shubhrajyoti Datta wrote:
> Add the EIO GPIO support.
> Add the dt description and the compatible to the driver.
>
> Changes in v4:
> - Add Conor ack
> - Remove the min 52 for eio
>
> [...]
Applied, thanks!
[1/3] dt-bindings: gpio: zynq: Sort compatible strings alphabetically
https://git.kernel.org/brgl/c/3eb639ef8da2d418ae69f3c8840c4e815036adc6
[2/3] dt-bindings: gpio: Add EIO GPIO compatible to gpio-zynq
https://git.kernel.org/brgl/c/18409d06b4a002cb8550ad7c20273bedc77851df
[3/3] gpio: zynq: Add eio gpio support
https://git.kernel.org/brgl/c/eeb1d6dfd89344b17afe845d4839b79e37fdd547
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH 08/10] clk: amlogic: Add A9 PLL clock controller driver
From: Jian Hu @ 2026-05-13 7:25 UTC (permalink / raw)
To: Brian Masney
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Neil Armstrong, Jerome Brunet, Xianwei Zhao,
Kevin Hilman, Martin Blumenstingl, linux-kernel, linux-clk,
devicetree, linux-amlogic, linux-arm-kernel
In-Reply-To: <agH3f3F0pUNOzdPB@redhat.com>
On 5/11/2026 11:36 PM, Brian Masney wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Jian,
>
> On Mon, May 11, 2026 at 08:47:30PM +0800, Jian Hu via B4 Relay wrote:
>> From: Jian Hu <jian.hu@amlogic.com>
>>
>> Add the PLL clock controller driver for the Amlogic A9 SoC family.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>> drivers/clk/meson/Kconfig | 13 +
>> drivers/clk/meson/Makefile | 1 +
>> drivers/clk/meson/a9-pll.c | 831 +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 845 insertions(+)
>>
>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>> index cf8cf3f9e4ee..3549e67d6988 100644
>> --- a/drivers/clk/meson/Kconfig
>> +++ b/drivers/clk/meson/Kconfig
>> @@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>> controller to work.
>>
>> +config COMMON_CLK_A9_PLL
>> + tristate "Amlogic A9 SoC PLL controller support"
>> + depends on ARM64
> depends on ARM64 || COMPILE_TEST
Ok, I will add COMPILE_TEST in the next version.
>> + default ARCH_MESON
>> + select COMMON_CLK_MESON_REGMAP
>> + select COMMON_CLK_MESON_CLKC_UTILS
>> + select COMMON_CLK_MESON_PLL
>> + imply COMMON_CLK_SCMI
>> + help
>> + Support for the PLL clock controller on Amlogic A311Y3 based
>> + device, AKA A9. PLLs are required by most peripheral to operate.
>> + Say Y if you want A9 PLL clock controller to work.
>> +
>> config COMMON_CLK_C3_PLL
>> tristate "Amlogic C3 PLL clock controller"
>> depends on ARM64
>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>> index c6719694a242..77636033061f 100644
>> --- a/drivers/clk/meson/Makefile
>> +++ b/drivers/clk/meson/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>> +obj-$(CONFIG_COMMON_CLK_A9_PLL) += a9-pll.o
>> obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
>> obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>> diff --git a/drivers/clk/meson/a9-pll.c b/drivers/clk/meson/a9-pll.c
>> new file mode 100644
>> index 000000000000..84b591c3afff
>> --- /dev/null
>> +++ b/drivers/clk/meson/a9-pll.c
>> @@ -0,0 +1,831 @@
>> +// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
>> +/*
>> + * Copyright (C) 2026 Amlogic, Inc. All rights reserved
>> + */
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>> +#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
>> +#include "clk-regmap.h"
>> +#include "clk-pll.h"
>> +#include "meson-clkc-utils.h"
> Sort the headers
Ok , I will place dt-bindings header at the top.
After updated:
#include <dt-bindings/clock/amlogic,a9-pll-clkc.h>
#include <linux/clk-provider.h>
#include <linux/platform_device.h>
#include "clk-regmap.h"
#include "clk-pll.h"
#include "meson-clkc-utils.h"
If I have misunderstood, please correct me.
>> +
>> +#define GP0PLL_CTRL0 0x00
>> +#define GP0PLL_CTRL1 0x04
>> +#define GP0PLL_CTRL2 0x08
>> +#define GP0PLL_CTRL3 0x0c
>> +#define GP0PLL_CTRL4 0x10
>> +
>> +/* HIFI0 and HIFI1 share the same IP and register offset layout. */
>> +#define HIFIPLL_CTRL0 0x00
>> +#define HIFIPLL_CTRL1 0x04
>> +#define HIFIPLL_CTRL2 0x08
>> +#define HIFIPLL_CTRL3 0x0c
>> +#define HIFIPLL_CTRL4 0x10
>> +
>> +/* MCLK0 and MCLK1 share the same IP and register offset layout. */
>> +#define MCLKPLL_CTRL0 0x00
>> +#define MCLKPLL_CTRL1 0x04
>> +#define MCLKPLL_CTRL2 0x08
>> +#define MCLKPLL_CTRL3 0x0c
>> +#define MCLKPLL_CTRL4 0x10
>> +
>> +#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
>> + MESON_COMP_SEL(a9_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
>> +
>> +#define A9_COMP_DIV(_name, _reg, _shift, _width) \
>> + MESON_COMP_DIV(a9_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
>> +
>> +#define A9_COMP_GATE(_name, _reg, _bit) \
>> + MESON_COMP_GATE(a9_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
>> +
>> +/*
>> + * Compared with previous SoC PLLs, the A9 PLL input path has an inherent
>> + * 2-divider. The N pre-divider follows the same calculation rule as OD,
>> + * where the pre-divider ratio equals 2^N.
>> + *
>> + * A9 PLL is composed as follows:
>> + *
>> + * PLL
>> + * +---------------------------------+
>> + * | |
>> + * | +--+ |
>> + * in/2 >>---[ /2^N ]-->| | +-----+ |
>> + * | | |------| DCO |----->> out
>> + * | +--------->| | +--v--+ |
>> + * | | +--+ | |
>> + * | | | |
>> + * | +--[ *(M + (F/Fmax) ]<--+ |
>> + * | |
>> + * +---------------------------------+
>> + *
>> + * out = in / 2 * (m + frac / frac_max) / 2^n
>> + */
>> +
>> +static struct clk_fixed_factor a9_gp0_in_div2_div = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "gp0_in_div2_div",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_data = &(const struct clk_parent_data) {
>> + .fw_name = "in0",
>> + },
>> + .num_parents = 1,
>> + },
> You can use CLK_HW_INIT_FW_NAME() for the hw.init here and other places
> below.
Ok, I will use CLK_HW_INIT_FW_NAME instead in the next version.
>> +};
>> +
>> +static struct clk_regmap a9_gp0_in_div2 = {
>> + .data = &(struct clk_regmap_gate_data) {
>> + .offset = GP0PLL_CTRL0,
>> + .bit_idx = 27,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "gp0_in_div2",
>> + .ops = &clk_regmap_gate_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &a9_gp0_in_div2_div.hw
>> + },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +/* The output frequency range of the A9 PLL_DCO is 1.4 GHz to 2.8 GHz. */
>> +static const struct pll_mult_range a9_pll_mult_range = {
>> + .min = 117,
>> + .max = 233,
>> +};
>> +
>> +static const struct reg_sequence a9_gp0_pll_init_regs[] = {
>> + { .reg = GP0PLL_CTRL0, .def = 0x00010000 },
>> + { .reg = GP0PLL_CTRL1, .def = 0x11480000 },
>> + { .reg = GP0PLL_CTRL2, .def = 0x1219b010 },
>> + { .reg = GP0PLL_CTRL3, .def = 0x00008010 }
>> +};
>> +
>> +static struct clk_regmap a9_gp0_pll_dco = {
>> + .data = &(struct meson_clk_pll_data) {
>> + .en = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 0,
>> + .width = 9,
>> + },
>> + .n = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 12,
>> + .width = 3,
>> + },
>> + .frac = {
>> + .reg_off = GP0PLL_CTRL1,
>> + .shift = 0,
>> + .width = 17,
>> + },
>> + .l = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .l_detect = {
>> + .reg_off = GP0PLL_CTRL0,
>> + .shift = 30,
>> + .width = 1,
>> + },
>> + .range = &a9_pll_mult_range,
>> + .init_regs = a9_gp0_pll_init_regs,
>> + .init_count = ARRAY_SIZE(a9_gp0_pll_init_regs),
>> + .flags = CLK_MESON_PLL_RST_ACTIVE_LOW |
>> + CLK_MESON_PLL_N_POWER_OF_TWO |
>> + CLK_MESON_PLL_L_DETECT_ACTIVE_HIGH,
>> + },
>> + .hw.init = &(struct clk_init_data) {
>> + .name = "gp0_pll_dco",
>> + .ops = &meson_clk_pll_ops,
>> + .parent_hws = (const struct clk_hw *[]) {
>> + &a9_gp0_in_div2.hw
>> + },
>> + .num_parents = 1,
>> + },
> You can use CLK_HW_INIT_HWS() here and other places below.
>
> Brian
>
Ok, I will use CLK_HW_INIT_HW instead for single parent case.
Best regards,
Jian
[......]
>> 2.47.1
>>
>>
^ permalink raw reply
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Pali Rohár @ 2026-05-13 7:20 UTC (permalink / raw)
To: Hans Zhang
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <581e91fb-2e57-43ed-b79d-19dbf384b955@163.com>
On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
>
>
> On 5/13/26 05:25, Pali Rohár wrote:
> > On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
> > > The Aardvark PCIe controller driver waits for the link to come up but
> > > does not implement the mandatory 100 ms delay after link training
> > > completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
> > >
> > > The driver already maintains a 'link_gen' field that holds the negotiated
> > > link speed. Use it together with pcie_wait_after_link_train() to insert
> > > the required delay immediately after confirming that the link is up.
> > >
> > > Signed-off-by: Hans Zhang <18255117159@163.com>
> > > ---
> > > drivers/pci/controller/pci-aardvark.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index e34bea1ff0ac..526351c21c49 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
> > > /* check if the link is up or not */
> > > for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> > > - if (advk_pcie_link_up(pcie))
> > > + if (advk_pcie_link_up(pcie)) {
> > > + pcie_wait_after_link_train(pcie->link_gen);
> > > return 0;
> > > + }
> > > usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
> > > }
> > > --
> > > 2.34.1
> > >
> >
> > Are you sure that this is correct to do? Have you checked the A3720
> > Functional Specification which describes how to bring PCIe link up?
> >
> > A3720 PCIe controller is buggy and needs more timing hacks to make it
> > behave. Playing with random sleeps can break its internal logic.
> > I'm not sure if it could be safe without proper testing.
> >
> > And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
>
>
> Hi Pali,
>
> 1. This driver does not support A3720.
>
> static const struct of_device_id advk_pcie_of_match_table[] = {
> { .compatible = "marvell,armada-3700-pcie", },
> {},
> };
> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
>
> If you need support for A3720, please submit the corresponding patch so that
> Bjorn and Mani can review it.
3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
>
> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
> in the DT. This will not affect the functionality of this patch.
Whole A37xx supports only GEN2. And in DT files for 37xx should be
already there max-link-speed.
Seems that in advk_pcie_of_match_table there is no GEN3 device
specified.
> 3. This patch is a common delay requirement stipulated by the PCIe
> specification. If it is greater than GEN2, then msleep(100) will be added;
> otherwise, there will be no such delay.
>
> 4. For instance, we often come across the situation where some common APIs
> are modified, and in many cases, their functionality does not require the
> actual development board for verification. I believe that many other
> developers and maintainers have modified different parts of the code. For
> example, the recent submission:
Switching one API to another is one thing. But changing code which looks
to be critical, specially when it is known that hw has bugs, can cause
breaking of existing boards.
> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
> Author: Nam Cao <namcao@linutronix.de>
> Date: Thu Jun 26 16:47:53 2025 +0200
>
> PCI: aardvark: Switch to msi_create_parent_irq_domain()
>
> Switch to msi_create_parent_irq_domain() from
> pci_msi_create_irq_domain()
> which was using legacy MSI domain setup.
>
>
> And many controller drivers have been modified.
>
>
> Best regards,
> Hans
>
>
^ permalink raw reply
* Re: [PATCH v4 2/2] coco: guest: arm64: Drop dummy RSI platform device stub
From: Greg KH @ 2026-05-13 7:11 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Catalin Marinas, linux-kernel, linux-arm-kernel, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Jonathan Cameron, Suzuki K Poulose
In-Reply-To: <yq5a7bp7u77f.fsf@kernel.org>
On Wed, May 13, 2026 at 12:28:12PM +0530, Aneesh Kumar K.V wrote:
> Catalin Marinas <catalin.marinas@arm.com> writes:
>
> > + Suzuki again
> >
> > On Mon, Apr 27, 2026 at 11:46:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
> >> The SMCCC firmware driver now creates the `arm-smccc` platform device
> >> and also creates the CCA auxiliary devices once the RSI ABI is
> >> discovered. This makes the arch-specific arm64_create_dummy_rsi_dev()
> >> helper redundant. Remove the arm-cca-dev platform device registration
> >> and let the SMCCC probe manage the RSI device.
> >>
> >> systemd match on platform:arm-cca-dev for confidential vm detection [1].
> >> Losing the platform device registration can break that. Keeping this
> >> removal in its own change makes it easy to revert if that regression
> >> blocks the rollout.
> >>
> >> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
> >
> > I wouldn't merge this now given that systemd checks this file. Could we
> > have a symbolic link instead for some time until systemd eventually gets
> > updated (years?).
> >
>
> I’ll add this in the next revision.
>
> static int create_rsi_compat_link(struct device *target_dev)
> {
> struct kobject *platform_kobj;
> /*
> * target_dev is:
> * /sys/devices/platform/arm-smccc/arm_cca_guest.arm-rsi-dev.0
> * Create compat link /sys/devices/platform/arm-cca-dev
> */
> platform_kobj = target_dev->kobj.parent->parent;
What? That is crazy, you don't know that is always going to be ok.
> return sysfs_create_link(platform_kobj,
> &target_dev->kobj,
> "arm-cca-dev");
No, don't do that, if a driver calls a sysfs* function, something is
almost always wrong. Don't be making random sysfs symlinks please.
If userspace can not find the device anymore, that's fine, that's how
sysfs works, devices move around all the time. Especially platform
devices as those are almost always not supposed to be platform devices :)
thanks,
greg k-h
^ permalink raw reply
* [PATCH] media: rc: sunxi-cir: unregister rc device on probe failure
From: 박명훈 @ 2026-05-13 7:11 UTC (permalink / raw)
To: Sean Young, Mauro Carvalho Chehab
Cc: Myeonghun Pak, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
linux-media, linux-arm-kernel, linux-sunxi, linux-kernel, stable,
Ijae Kim
From: Myeonghun Pak <mhun512@gmail.com>
After rc_register_device() succeeds, later probe failures must undo the
registration with rc_unregister_device(). The current error path jumps to
the allocation cleanup label and only calls rc_free_device(), leaving the
rc device registration and resources created by rc_register_device()
behind.
Add a registered-device unwind label for the IRQ lookup, IRQ request, and
hardware initialization failure paths. Keep rc_free_device() for failures
before rc_register_device() succeeds.
Fixes: b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Cc: stable@vger.kernel.org
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
drivers/media/rc/sunxi-cir.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 92ef4e7c6f..cc64a68dfe 100644
--- a/drivers/media/rc/sunxi-cir.c
+++ b/drivers/media/rc/sunxi-cir.c
@@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
ir->irq = platform_get_irq(pdev, 0);
if (ir->irq < 0) {
ret = ir->irq;
- goto exit_free_dev;
+ goto exit_unregister_dev;
}
ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
if (ret) {
dev_err(dev, "failed request irq\n");
- goto exit_free_dev;
+ goto exit_unregister_dev;
}
ret = sunxi_ir_hw_init(dev);
if (ret)
- goto exit_free_dev;
+ goto exit_unregister_dev;
dev_info(dev, "initialized sunXi IR driver\n");
return 0;
+exit_unregister_dev:
+ rc_unregister_device(ir->rc);
+ return ret;
+
exit_free_dev:
rc_free_device(ir->rc);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training
From: Hans Zhang @ 2026-05-13 7:00 UTC (permalink / raw)
To: Pali Rohár
Cc: bhelgaas, lpieralisi, kwilczynski, mani, vigneshr, jingoohan1,
thomas.petazzoni, ryder.lee, jianjun.wang, claudiu.beznea.uj,
mpillai, robh, s-vadapalli, linux-omap, linux-arm-kernel,
linux-mediatek, linux-renesas-soc, linux-pci, linux-kernel
In-Reply-To: <20260512212531.jupoocz7acv22qyg@pali>
On 5/13/26 05:25, Pali Rohár wrote:
> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>> The Aardvark PCIe controller driver waits for the link to come up but
>> does not implement the mandatory 100 ms delay after link training
>> completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).
>>
>> The driver already maintains a 'link_gen' field that holds the negotiated
>> link speed. Use it together with pcie_wait_after_link_train() to insert
>> the required delay immediately after confirming that the link is up.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>> drivers/pci/controller/pci-aardvark.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>> index e34bea1ff0ac..526351c21c49 100644
>> --- a/drivers/pci/controller/pci-aardvark.c
>> +++ b/drivers/pci/controller/pci-aardvark.c
>> @@ -350,8 +350,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
>>
>> /* check if the link is up or not */
>> for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
>> - if (advk_pcie_link_up(pcie))
>> + if (advk_pcie_link_up(pcie)) {
>> + pcie_wait_after_link_train(pcie->link_gen);
>> return 0;
>> + }
>>
>> usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>> }
>> --
>> 2.34.1
>>
>
> Are you sure that this is correct to do? Have you checked the A3720
> Functional Specification which describes how to bring PCIe link up?
>
> A3720 PCIe controller is buggy and needs more timing hacks to make it
> behave. Playing with random sleeps can break its internal logic.
> I'm not sure if it could be safe without proper testing.
>
> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
Hi Pali,
1. This driver does not support A3720.
static const struct of_device_id advk_pcie_of_match_table[] = {
{ .compatible = "marvell,armada-3700-pcie", },
{},
};
MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
If you need support for A3720, please submit the corresponding patch so
that Bjorn and Mani can review it.
2. If A3720 only supports GEN2, you can configure "max-link-speed" to be
2 in the DT. This will not affect the functionality of this patch.
3. This patch is a common delay requirement stipulated by the PCIe
specification. If it is greater than GEN2, then msleep(100) will be
added; otherwise, there will be no such delay.
4. For instance, we often come across the situation where some common
APIs are modified, and in many cases, their functionality does not
require the actual development board for verification. I believe that
many other developers and maintainers have modified different parts of
the code. For example, the recent submission:
commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
Author: Nam Cao <namcao@linutronix.de>
Date: Thu Jun 26 16:47:53 2025 +0200
PCI: aardvark: Switch to msi_create_parent_irq_domain()
Switch to msi_create_parent_irq_domain() from
pci_msi_create_irq_domain()
which was using legacy MSI domain setup.
And many controller drivers have been modified.
Best regards,
Hans
^ permalink raw reply
* Re: [PATCH v4 2/2] coco: guest: arm64: Drop dummy RSI platform device stub
From: Aneesh Kumar K.V @ 2026-05-13 6:58 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-kernel, linux-arm-kernel, Greg KH, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Jonathan Cameron, Suzuki K Poulose
In-Reply-To: <agM7VmoQ--ylUmyM@arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> + Suzuki again
>
> On Mon, Apr 27, 2026 at 11:46:15AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> The SMCCC firmware driver now creates the `arm-smccc` platform device
>> and also creates the CCA auxiliary devices once the RSI ABI is
>> discovered. This makes the arch-specific arm64_create_dummy_rsi_dev()
>> helper redundant. Remove the arm-cca-dev platform device registration
>> and let the SMCCC probe manage the RSI device.
>>
>> systemd match on platform:arm-cca-dev for confidential vm detection [1].
>> Losing the platform device registration can break that. Keeping this
>> removal in its own change makes it easy to revert if that regression
>> blocks the rollout.
>>
>> [1] https://lore.kernel.org/all/4a7d84b2-2ec4-4773-a2d5-7b63d5c683cf@arm.com
>
> I wouldn't merge this now given that systemd checks this file. Could we
> have a symbolic link instead for some time until systemd eventually gets
> updated (years?).
>
I’ll add this in the next revision.
static int create_rsi_compat_link(struct device *target_dev)
{
struct kobject *platform_kobj;
/*
* target_dev is:
* /sys/devices/platform/arm-smccc/arm_cca_guest.arm-rsi-dev.0
* Create compat link /sys/devices/platform/arm-cca-dev
*/
platform_kobj = target_dev->kobj.parent->parent;
return sysfs_create_link(platform_kobj,
&target_dev->kobj,
"arm-cca-dev");
}
-aneesh
^ permalink raw reply
* Re: [PATCH v4 1/2] firmware: smccc: coco: Manage arm-smccc platform device and CCA auxiliary drivers
From: Aneesh Kumar K.V @ 2026-05-13 6:56 UTC (permalink / raw)
To: Catalin Marinas
Cc: linux-kernel, linux-arm-kernel, Greg KH, Jeremy Linton,
Jonathan Cameron, Lorenzo Pieralisi, Mark Rutland, Sudeep Holla,
Will Deacon, Suzuki K Poulose
In-Reply-To: <agM699d4KwdjnS39@arm.com>
Catalin Marinas <catalin.marinas@arm.com> writes:
> + Suzuki
>
> On Mon, Apr 27, 2026 at 11:46:14AM +0530, Aneesh Kumar K.V (Arm) wrote:
>> diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
>> index 88b50d660e85..2d2d363aaaee 100644
>> --- a/arch/arm64/include/asm/rsi.h
>> +++ b/arch/arm64/include/asm/rsi.h
>> @@ -10,7 +10,7 @@
>> #include <linux/jump_label.h>
>> #include <asm/rsi_cmds.h>
>>
>> -#define RSI_PDEV_NAME "arm-cca-dev"
>> +#define RSI_DEV_NAME "arm-rsi-dev"
> [...]
>> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
>> index bdee057db2fd..fc9b44b7c687 100644
>> --- a/drivers/firmware/smccc/smccc.c
>> +++ b/drivers/firmware/smccc/smccc.c
>> @@ -12,6 +12,8 @@
>> #include <linux/platform_device.h>
>> #include <asm/archrandom.h>
>>
>> +#include "rmm.h"
>> +
>> static u32 smccc_version = ARM_SMCCC_VERSION_1_0;
>> static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
>>
>> @@ -85,6 +87,18 @@ static int __init smccc_devices_init(void)
>> {
>> struct platform_device *pdev;
>>
>> + pdev = platform_device_register_simple("arm-smccc",
>> + PLATFORM_DEVID_NONE, NULL, 0);
>> + if (IS_ERR(pdev)) {
>> + pr_err("arm-smccc: could not register device: %ld\n", PTR_ERR(pdev));
>> + } else {
>> + /*
>> + * Register the RMI and RSI devices only when firmware exposes
>> + * the required SMCCC function IDs at a supported revision.
>> + */
>> + register_rsi_device(pdev);
>> + }
>
> So as per the cover letter, instead of "arm-cca-dev" as a platform
> device, we get "arm-smccc" as a platform device with an auxiliary
> "arm-rsi-dev" child device. This does not get rid of the platform
> device, it just creates a synthetic platform device to represent the
> SMCCC firmware interface.
>
> Looking at the earlier discussion, I think this is what Greg/Jason were
> suggesting, except that we do not currently have an SMCCC platform
> device:
>
> https://lore.kernel.org/all/2025101534-frosty-shank-00b1@gregkh/
>
> If we go this route, shouldn't the platform device above be created only
> if !SMCCC_CONDUIT_NONE?
>
register_rsi_device() does check for
if (arm_smccc_1_1_get_conduit() != SMCCC_CONDUIT_SMC)
return;
>
> "smccc_trng" would also fit this model (together with the driver),
> assuming we don't break any user-space (searching the Debian codebase
> did not find any use).
Will switch smccc_trng to an auxiliary device in the next revision.
-aneesh
^ permalink raw reply
* Re: [PATCH v4 01/10] dt-bindings: display: rockchip: analogix-dp: Allow hclk as third clock
From: Damon Ding @ 2026-05-13 6:48 UTC (permalink / raw)
To: Conor Dooley
Cc: hjc, heiko, andy.yan, maarten.lankhorst, mripard, tzimmermann,
airlied, simona, robh, krzk+dt, conor+dt, andrzej.hajda,
neil.armstrong, rfoss, Laurent.pinchart, jonas, jernej.skrabec,
nicolas.frattaroli, cristian.ciocaltea, sebastian.reichel,
dmitry.baryshkov, luca.ceresoli, dianders, m.szyprowski,
dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel
In-Reply-To: <20260512-diabetic-ahead-dd36bc2d8be7@spud>
Hi Conor,
On 5/13/2026 1:12 AM, Conor Dooley wrote:
> On Tue, May 12, 2026 at 05:56:35PM +0800, Damon Ding wrote:
>> RK3588 eDP controller requires HCLK_VO1 (video output bus clock)
>> to access the VO1 GRF registers and enable the video datapath.
>>
>> Previously, the clock was enabled implicitly via the 'rockchip,vo-grf'
>> phandle reference, which allowed the eDP to work without explicitly
>> managing the hclk_vo1 clock. However, this is not safe or explicit.
>>
>> To align with other display controllers (HDMI) on RK3588 and make
>> the clock requirement explicit, expand clock-names to support either
>> "grf" (for older SoCs) or "hclk" (for RK3588) as the third clock.
>>
>> This makes the clock dependency clear and removes reliance on implicit
>> clock enablement from GRF phandle.
>>
>> Fixes: f855146263b1 ("dt-bindings: display: rockchip: analogix-dp: Add support for RK3588")
>> Signed-off-by: Damon Ding <damon.ding@rock-chips.com>
>>
>> ---
>>
>> Changes in v4:
>> - Modify the commit msg.
>> ---
>> .../bindings/display/rockchip/rockchip,analogix-dp.yaml | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> index d99b23b88cc5..d2bc8636b626 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,analogix-dp.yaml
>> @@ -26,7 +26,9 @@ properties:
>> items:
>> - const: dp
>> - const: pclk
>> - - const: grf
>> + - enum:
>> + - grf
>> + - hclk
>
> Could you also enforce the correct clock name on a per-compatible basis
> please?
>
> pw-bot: changes-requested
>
Yes, will do in v5.
>
>>
>> power-domains:
>> maxItems: 1
>> --
>> 2.34.1
>>
Best regards,
Damon
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: ti: icssg: Derive stats array lengths from ARRAY_SIZE
From: MD Danish Anwar @ 2026-05-13 6:29 UTC (permalink / raw)
To: David CARLIER
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Roger Quadros,
Andrew Lunn, Jacob Keller, Meghana Malladi, Kevin Hao,
Vadim Fedorenko, netdev, linux-doc, linux-kernel,
linux-arm-kernel, Vignesh Raghavendra
In-Reply-To: <CA+XhMqzx9CUX5H7q1UqL=heGWLFjZVfyiTx6b45VW=E9t13Fow@mail.gmail.com>
Hi David
On 12/05/26 3:33 pm, David CARLIER wrote:
> Hi Danish,
>
>
> On Tue, 12 May 2026 at 10:40, MD Danish Anwar <danishanwar@ti.com> wrote:
>>
>> Hi David,
>>
>> On 12/05/26 1:28 pm, David CARLIER wrote:
>>> Hi MD,
>>>
>>> On Tue, 12 May 2026 at 07:06, MD Danish Anwar <danishanwar@ti.com> wrote:
>>>>
>>>> Replace the manually maintained ICSSG_NUM_MIIG_STATS and
>>>> ICSSG_NUM_PA_STATS constants with ARRAY_SIZE() expressions derived
>>>> directly from the corresponding stat descriptor arrays, so that adding
>>>> new entries to icssg_all_miig_stats[] or icssg_all_pa_stats[] no longer
>>>> requires a separate update to a numeric constant.
>>>>
>>>> To make this self-contained, break the circular include dependency
>>>> between icssg_stats.h and icssg_prueth.h:
>>>>
>>>> - icssg_stats.h previously included icssg_prueth.h (transitively
>>>> pulling in icssg_switch_map.h and ETH_GSTRING_LEN). Replace that
>>>> with direct includes of <linux/ethtool.h>, <linux/kernel.h> and
>>>> "icssg_switch_map.h".
>>>>
>>>> - icssg_prueth.h now includes icssg_stats.h, giving it access to
>>>> the ARRAY_SIZE-based ICSSG_NUM_MIIG_STATS and ICSSG_NUM_PA_STATS
>>>> before they are used in the prueth_emac struct and ICSSG_NUM_STATS.
>>>>
>>>> Signed-off-by: MD Danish Anwar <danishanwar@ti.com>
>>>> ---
>>>> drivers/net/ethernet/ti/icssg/icssg_prueth.h | 3 +--
>>>> drivers/net/ethernet/ti/icssg/icssg_stats.h | 7 ++++++-
>>>> 2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> index df93d15c5b78..e2ccecb0a0dd 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>>>> @@ -43,6 +43,7 @@
>>>>
>>>> #include "icssg_config.h"
>>>> #include "icss_iep.h"
>>>> +#include "icssg_stats.h"
>>>> #include "icssg_switch_map.h"
>>>>
>>>> #define PRUETH_MAX_MTU (2000 - ETH_HLEN - ETH_FCS_LEN)
>>>> @@ -57,8 +58,6 @@
>>>>
>>>> #define ICSSG_MAX_RFLOWS 8 /* per slice */
>>>>
>>>> -#define ICSSG_NUM_PA_STATS 32
>>>> -#define ICSSG_NUM_MIIG_STATS 60
>>>> /* Number of ICSSG related stats */
>>>> #define ICSSG_NUM_STATS (ICSSG_NUM_MIIG_STATS + ICSSG_NUM_PA_STATS)
>>>> #define ICSSG_NUM_STANDARD_STATS 31
>>>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_stats.h b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> index 5ec0b38e0c67..b854eb587c1e 100644
>>>> --- a/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> +++ b/drivers/net/ethernet/ti/icssg/icssg_stats.h
>>>> @@ -8,10 +8,15 @@
>>>> #ifndef __NET_TI_ICSSG_STATS_H
>>>> #define __NET_TI_ICSSG_STATS_H
>>>>
>>>> -#include "icssg_prueth.h"
>>>> +#include <linux/ethtool.h>
>>>> +#include <linux/kernel.h>
>>>> +#include "icssg_switch_map.h"
>>>>
>>>> #define STATS_TIME_LIMIT_1G_MS 25000 /* 25 seconds @ 1G */
>>>>
>>>> +#define ICSSG_NUM_MIIG_STATS ARRAY_SIZE(icssg_all_miig_stats)
>>>> +#define ICSSG_NUM_PA_STATS ARRAY_SIZE(icssg_all_pa_stats)
>>>> +
>>>> struct miig_stats_regs {
>>>> /* Rx */
>>>> u32 rx_packets;
>>>> --
>>>> 2.34.1
>>>>
>>>
>>> One thing that caught my eye: icssg_all_miig_stats[] and
>>> icssg_all_pa_stats[] are 'static const' arrays in icssg_stats.h with
>>> ETH_GSTRING_LEN name buffers per entry. Right now only icssg_stats.c
>>> and icssg_ethtool.c pull them in. After this patch icssg_prueth.h
>>> includes icssg_stats.h, so every .c in the driver (classifier,
>>> common, config, mii_cfg, queues, switchdev, ...) ends up with its own
>>> static-const copy of both tables.
>>>
>>> Would a static_assert() work for what you're after? Something like:
>>>
>>
>> While adding more stats manually, The ARRAY_SIZE() approach was
>> explicitly requested by maintainer [1]:
>>
>> This patch is a direct response to that feedback. static_assert() would
>> still require updating the numeric constant on every array change. The
>> goal here is to eliminate the need of manually incrementing stats count
>> whenever new stats are added
>>
>> Your concern about multiple copies of table is noted and valid. Could
>> you advise on the preferred way to reconcile these two requirements? I
>> am happy to restructure if there is an approach that satisfies both.
>>
>> [1]
>> https://lore.kernel.org/all/20260112181436.4s5ceywwembn674r@skbuf/#:~:text=Can%27t%20this%20be%20expressed%20as%20ARRAY_SIZE(icssg_all_pa_stats)%3F%20It%20is%20very%0Afragile%20to%20have%20to%20count%20and%20update%20this%20manually.
>>
>>
>>> static const struct icssg_miig_stats icssg_all_miig_stats[] = {
>>> ...
>>> };
>>> static_assert(ARRAY_SIZE(icssg_all_miig_stats) == ICSSG_NUM_MIIG_STATS);
>>>
>>> next to each array, keeping the numeric #defines as-is. Then 2/2 fails
>>> to build the moment a new entry is added without bumping the count,
>>> which is the case you're guarding against — without touching the
>>> include graph.
>>>
>>> What do you think ?
>>>
>>> Cheers.
>>
>> --
>> Thanks and Regards,
>> Danish
>>
>
>
> Thanks for digging up the context — fair point, I'd missed Vladimir's
> earlier ask. Reading it again though, what he calls fragile is the
> silent miscount, not the keystroke of typing a number. A static_assert
> turns "forgot to bump" into a build error, which I think gets you
> there.
>
Thank you for the suggestion. I think your previous suggestion fits
better. I believe keeping the arrays in icssg_stats.h is preferable to
moving them to icssg_stats.c. Here is my reasoning:
Your binary-bloat concern was about icssg_prueth.h including
icssg_stats.h, which would drag the static const tables into every .c
that includes icssg_prueth.h (~11 translation units). That concern is
valid, but it is specific to the include direction of the previous
patch. If we simply revert to the original include graph —
icssg_stats.h includes icssg_prueth.h, not the other way around —
only the two files that have always included icssg_stats.h directly
(icssg_stats.c and icssg_ethtool.c) get a copy of the arrays. No
regression in binary size compared to the baseline.
> What about moving the two arrays into icssg_stats.c, declaring them
> extern in the header, and dropping a static_assert next to each
> definition? Numeric #defines stay where they are, icssg_prueth.h
> doesn't need to know about icssg_stats.h, and the tables live in one
> TU instead of every .o in the driver. If the count and the array
> disagree, you get a compile error on the spot.
>
Moving the arrays to icssg_stats.c (approach #2) adds extern
declarations, splits the definition from the static_assert, and is a
larger restructuring for the same safety guarantee. Keeping the arrays
in the header with a static_assert immediately after each one is a
2-line diff and leaves the code easy to read in one place.
Please let me know if this sounds okay to you. I will send out a v2 soon
if this approach is fine with you.
> Probably worth keeping Vladimir on Cc for v2 in case he had something
> else in mind.
>
I will CC Vladimir in v2.
--
Thanks and Regards,
Danish
^ permalink raw reply
* Re: [PATCH 3/4] remoteproc: add helper for optional ELF resource tables
From: Daniel Baluta @ 2026-05-13 6:30 UTC (permalink / raw)
To: tanmay.shah, Ben Levinsky, Bjorn Andersson, Mathieu Poirier,
linux-remoteproc
Cc: Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Geert Uytterhoeven, Magnus Damm, Patrice Chotard, Maxime Coquelin,
Alexandre Torgue, imx, linux-arm-kernel, linux-kernel,
linux-renesas-soc, linux-stm32
In-Reply-To: <9e3a88d9-1679-43ae-a96b-62a29bd45d9d@amd.com>
On 5/12/26 17:53, Shah, Tanmay wrote:
>
>
> On 5/12/2026 2:55 AM, Daniel Baluta wrote:
>> On 5/12/26 00:18, Ben Levinsky wrote:
>>> [You don't often get email from ben.levinsky@amd.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Add a small helper around rproc_elf_load_rsc_table() for remoteproc
>>> drivers that treat a missing ELF resource table as optional. The helper
>>> returns success on -EINVAL and propagates other failures unchanged.
>>>
>>> Signed-off-by: Ben Levinsky <ben.levinsky@amd.com>
>>> ---
>>> drivers/remoteproc/remoteproc_internal.h | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
>>> index 3724a47a9748..dff87e468837 100644
>>> --- a/drivers/remoteproc/remoteproc_internal.h
>>> +++ b/drivers/remoteproc/remoteproc_internal.h
>>> @@ -146,6 +146,18 @@ static inline int rproc_mem_entry_iounmap(struct rproc *rproc,
>>> return 0;
>>> }
>>>
>>> +static inline int rproc_elf_load_rsc_table_optional(struct rproc *rproc,
>>> + const struct firmware *fw)
>>> +{
>>> + int ret;
>>> +
>>> + ret = rproc_elf_load_rsc_table(rproc, fw);
>>> + if (ret == -EINVAL)
>>> + dev_dbg(&rproc->dev, "no resource table found\n");
>>
>> You are changing loglevel here. Initial drivers use dev_info or dev_warn. At least I'm used
>> with seeing this messages in the logs.
>>
>> So, what do you think on adding at least dev_info to this instead of dev_dbg?
>>
>
> Actually can we leave that choice to the platform driver ? There are
> many use cases where the remoteproc subsystem is used to load and start
> the remote core and the firmware doesn't have the resource table. We
> don't want to make info level log for such use cases, as the resource
> table is not expected in the first place there.
Agree, this is the best way to go.
^ permalink raw reply
* [PATCH v2 3/3] clk: nuvoton: ma35d1: fix ma35d1_clk_pll_determine_rate logic
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
ma35d1_clk_pll_determine_rate() called ma35d1_pll_find_closest()
unconditionally before the switch statement, and then every case
branch overwrote pll_freq by reading the current hardware registers.
For CAPLL and DDRPLL this means find_closest() ran unnecessarily
(and incorrectly, since those PLLs are read-only) and its result
was silently discarded.
Fix by moving the find_closest() call inside the APLL/EPLL/VPLL
branch where it belongs. Group CAPLL and DDRPLL together as
read-only PLLs that simply report their current rate; handle them
with an explicit if/else to keep the CAPLL (SMIC design) and DDRPLL
(standard design) paths distinct.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index 7e6b30d20c01..314b81e7727c 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -255,32 +255,32 @@ static int ma35d1_clk_pll_determine_rate(struct clk_hw *hw,
if (req->best_parent_rate < PLL_FREF_MIN_FREQ || req->best_parent_rate > PLL_FREF_MAX_FREQ)
return -EINVAL;
- ret = ma35d1_pll_find_closest(pll, req->rate, req->best_parent_rate,
- reg_ctl, &pll_freq);
- if (ret < 0)
- return ret;
-
switch (pll->id) {
case CAPLL:
+ case DDRPLL:
+ /* Read-only PLLs: return current rate */
reg_ctl[0] = readl_relaxed(pll->ctl0_base);
- pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], req->best_parent_rate);
+ if (pll->id == CAPLL) {
+ pll_freq = ma35d1_calc_smic_pll_freq(reg_ctl[0], req->best_parent_rate);
+ } else {
+ reg_ctl[1] = readl_relaxed(pll->ctl1_base);
+ pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, req->best_parent_rate);
+ }
req->rate = pll_freq;
-
return 0;
- case DDRPLL:
case APLL:
case EPLL:
case VPLL:
- reg_ctl[0] = readl_relaxed(pll->ctl0_base);
- reg_ctl[1] = readl_relaxed(pll->ctl1_base);
- pll_freq = ma35d1_calc_pll_freq(pll->mode, reg_ctl, req->best_parent_rate);
+ /* Configurable PLLs: find closest achievable rate */
+ ret = ma35d1_pll_find_closest(pll, req->rate, req->best_parent_rate,
+ reg_ctl, &pll_freq);
+ if (ret < 0)
+ return ret;
req->rate = pll_freq;
-
return 0;
}
req->rate = 0;
-
return 0;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] clk: nuvoton: ma35d1: fix PLL_CTL1_FRAC bit field width and fractional calc
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
PLL_CTL1_FRAC was defined as GENMASK(31, 24), covering only 8 bits.
The hardware fractional field occupies bits [31:8] (24 bits), so the
mask must be GENMASK(31, 8).
The previous fractional-mode calculation used FIELD_MAX(PLL_CTL1_FRAC)
as the denominator to obtain 2 decimal places. With the corrected 24-bit
mask the old divisor is wrong; replace the arithmetic with a proper
24-bit fixed-point rounding to 3 decimal places:
n_frac = n * 1000 + (x * 1000 + 500) >> 24
The +500 term provides round-to-nearest before the right shift.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index bfedd45bd04b..7e6b30d20c01 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -48,7 +48,7 @@
#define PLL_CTL1_PD BIT(0)
#define PLL_CTL1_BP BIT(1)
#define PLL_CTL1_OUTDIV GENMASK(6, 4)
-#define PLL_CTL1_FRAC GENMASK(31, 24)
+#define PLL_CTL1_FRAC GENMASK(31, 8)
#define PLL_CTL2_SLOPE GENMASK(23, 0)
#define INDIV_MIN 1
@@ -113,9 +113,9 @@ static unsigned long ma35d1_calc_pll_freq(u8 mode, u32 *reg_ctl, unsigned long p
pll_freq = div_u64(pll_freq, m * p);
} else {
x = FIELD_GET(PLL_CTL1_FRAC, reg_ctl[1]);
- /* 2 decimal places floating to integer (ex. 1.23 to 123) */
- n = n * 100 + ((x * 100) / FIELD_MAX(PLL_CTL1_FRAC));
- pll_freq = div_u64(parent_rate * n, 100 * m * p);
+ /* x is 24-bit fractional part, convert to 3 decimal digits */
+ n = n * 1000 + (u32)(((u64)x * 1000 + 500) >> 24);
+ pll_freq = div_u64((u64)parent_rate * n, 1000 * m * p);
}
return pll_freq;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/3] clk: nuvoton: ma35d1: fix ignored div_u64 return values in PLL freq calculation
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
In-Reply-To: <20260513055626.1070533-1-a0987203069@gmail.com>
div_u64() does not modify its argument in place; the return value must
be assigned. Both ma35d1_calc_smic_pll_freq() and ma35d1_calc_pll_freq()
called div_u64() and discarded the result, leaving pll_freq holding the
undivided product and thus returning a frequency orders of magnitude too
high.
Fixes: 691521a367cf ("clk: nuvoton: Add clock driver for ma35d1 clock controller")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
---
drivers/clk/nuvoton/clk-ma35d1-pll.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/nuvoton/clk-ma35d1-pll.c b/drivers/clk/nuvoton/clk-ma35d1-pll.c
index 4620acfe47e8..bfedd45bd04b 100644
--- a/drivers/clk/nuvoton/clk-ma35d1-pll.c
+++ b/drivers/clk/nuvoton/clk-ma35d1-pll.c
@@ -92,7 +92,7 @@ static unsigned long ma35d1_calc_smic_pll_freq(u32 pll0_ctl0,
p = FIELD_GET(SPLL0_CTL0_OUTDIV, pll0_ctl0);
outdiv = 1 << p;
pll_freq = (u64)parent_rate * n;
- div_u64(pll_freq, m * outdiv);
+ pll_freq = div_u64(pll_freq, m * outdiv);
return pll_freq;
}
@@ -110,7 +110,7 @@ static unsigned long ma35d1_calc_pll_freq(u8 mode, u32 *reg_ctl, unsigned long p
if (mode == PLL_MODE_INT) {
pll_freq = (u64)parent_rate * n;
- div_u64(pll_freq, m * p);
+ pll_freq = div_u64(pll_freq, m * p);
} else {
x = FIELD_GET(PLL_CTL1_FRAC, reg_ctl[1]);
/* 2 decimal places floating to integer (ex. 1.23 to 123) */
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/3] clk: nuvoton: ma35d1: fix PLL frequency calculation
From: Joey Lu @ 2026-05-13 5:56 UTC (permalink / raw)
To: mturquette, sboyd
Cc: ychuang3, schung, yclu4, linux-arm-kernel, linux-clk,
linux-kernel, Joey Lu
Fix four bugs in the MA35D1 PLL clock driver that cause incorrect
frequency values returned from recalc_rate() and determine_rate().
v1 combined all fixes into a single commit. At reviewer request,
split into one patch per logical fix:
1/3 - fix div_u64 return value being discarded (affects both
ma35d1_calc_smic_pll_freq and ma35d1_calc_pll_freq INT mode)
2/3 - fix PLL_CTL1_FRAC mask width (8-bit -> 24-bit) and update
the fractional-mode arithmetic accordingly
3/3 - fix ma35d1_clk_pll_determine_rate: move find_closest() into
the configurable-PLL branch; unify read-only PLL handling
Joey Lu (3):
clk: nuvoton: ma35d1: fix ignored div_u64 return values in PLL freq
calculation
clk: nuvoton: ma35d1: fix PLL_CTL1_FRAC bit field width and fractional
calc
clk: nuvoton: ma35d1: fix ma35d1_clk_pll_determine_rate logic
drivers/clk/nuvoton/clk-ma35d1-pll.c | 38 ++++++++++++++--------------
1 file changed, 19 insertions(+), 19 deletions(-)
--
2.43.0
^ permalink raw reply
* RE: [PATCH 4/5] arm64: dts: freescale: imx952-evk: Add IMX-AUD-IO board support
From: Chancel Liu @ 2026-05-13 5:50 UTC (permalink / raw)
To: Bough Chen, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Frank Li, s.hauer@pengutronix.de,
festevam@gmail.com, mturquette@baylibre.com, sboyd@kernel.org
Cc: kernel@pengutronix.de, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org
In-Reply-To: <GV2PR04MB122722970C97CBA8B5FDFCFBF90382@GV2PR04MB12272.eurprd04.prod.outlook.com>
> > diff --git a/arch/arm64/boot/dts/freescale/imx952-evk.dts
> > b/arch/arm64/boot/dts/freescale/imx952-evk.dts
> > index 62d1c1c7c501..bb1d8d5f5fcf 100644
> > --- a/arch/arm64/boot/dts/freescale/imx952-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx952-evk.dts
> > @@ -43,6 +43,17 @@ aliases {
> > spi6 = &lpspi7;
> > };
> >
> > + aud_io_conn: aud-io-connector {
> > + compatible = "fsl,io-connector";
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-map = <0 0 &pcal6416 8 1>;
> > + gpio-map-mask = <0xff 0x0>;
> > + gpio-map-pass-thru = <0x0 0x1>;
>
> According to the include/dt-bindings/gpio/gpio.h, there 6 bits definition for
> GPIO flags, here you just pass through bit 0, should it better to use the
> following value:
> gpio-map-pass-thru = <0x0 0x3f>;
>
> Regards
> Haibo Chen
Good point.
For this connector on the base board, the intention is to only pass
through the polarity flag (bit0) in the GPIO flags cell. Other standard
GPIO flags are fixed on the base board side and won't be propagated
across this connector.
To avoid confusion, I'll add a short comment in the DT to explicitly
document that only the polarity bit is passed for GPIO flags on this
connector.
Regards,
Chancel Liu
^ permalink raw reply
* Re: [PATCH 01/19] btrfs: require at least 4 devices for RAID 6
From: Christoph Hellwig @ 2026-05-13 5:47 UTC (permalink / raw)
To: David Sterba
Cc: Christoph Hellwig, Andrew Morton, Catalin Marinas, Will Deacon,
Ard Biesheuvel, Huacai Chen, WANG Xuerui, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Herbert Xu, Dan Williams, Chris Mason, David Sterba,
Arnd Bergmann, Song Liu, Yu Kuai, Li Nan, linux-kernel,
linux-arm-kernel, loongarch, linuxppc-dev, linux-riscv,
linux-s390, linux-crypto, linux-btrfs, linux-arch, linux-raid
In-Reply-To: <20260512114231.GG2558453@suse.cz>
On Tue, May 12, 2026 at 01:42:31PM +0200, David Sterba wrote:
> On Tue, May 12, 2026 at 07:20:41AM +0200, Christoph Hellwig wrote:
> > While the RAID6 algorithm could in theory support 3 devices by just
> > copying the data disk to the two parity disks, this version is not only
> > useless because it is a suboptimal version of 3-way mirroring, but also
> > broken with various crashes and incorrect parity generation in various
> > architecture-optimized implementations. Disallow it similar to mdraid
> > which requires at least 4 devices for RAID 6.
> >
> > Fixes: 53b381b3abeb ("Btrfs: RAID5 and RAID6")
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> This patch should have been sent separately as it has user visible
> impact and can potentially break some setups.
It _is_ sent out separate.
> The degenerate modes of
> raid0, 5, or 6 are explicit as a possible middle step when converting
> profiles. We can use a fallback implementation for this case if the
> accelerated implementations cannot do it.
This is not about a degenerated mode. For a degenerated RAID 6, parity
generation uses the RAID 5 XOR routines as the second parity will be
missing. This is about generating two parities for a single data disk,
which must be explicitly selected.
^ permalink raw reply
* Re: [PATCH v3 06/13] pmdomain: core: Add initial fine grained sync_state support
From: Saravana Kannan @ 2026-05-13 5:34 UTC (permalink / raw)
To: Ulf Hansson
Cc: Saravana Kannan, Danilo Krummrich, Rafael J . Wysocki,
Greg Kroah-Hartman, driver-core, linux-pm, Sudeep Holla,
Cristian Marussi, Kevin Hilman, Stephen Boyd, Marek Szyprowski,
Bjorn Andersson, Abel Vesa, Peng Fan, Tomi Valkeinen, Maulik Shah,
Konrad Dybcio, Thierry Reding, Jonathan Hunter,
Geert Uytterhoeven, Dmitry Baryshkov, linux-arm-kernel,
linux-kernel
In-Reply-To: <CAPDyKFoxJ8AOyqzbEQc8x-e5fRf0p3faE23PRNcbA4aPSY_cdg@mail.gmail.com>
On Mon, May 11, 2026 at 3:25 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 11 May 2026 at 07:09, Saravana Kannan <saravanak@kernel.org> wrote:
> >
> > On Fri, May 8, 2026 at 5:39 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> > >
> > > A onecell (#power-domain-cells = <1 or 2>; in DT) power domain provider
> > > typically provides multiple independent power domains, each with their own
> > > corresponding consumers. In these cases we have to wait for all consumers
> > > for all the provided power domains before the ->sync_state() callback gets
> > > called for the supplier.
> > >
> > > In a first step to improve this, let's implement support for fine grained
> > > sync_state support a per genpd basis by using the ->queue_sync_state()
> > > callback. To take step by step, let's initially limit the improvement to
> > > the internal genpd provider driver and to its corresponding genpd devices
> > > for onecell providers.
> > >
> > > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> > > ---
> > >
> > > Changes in v3:
> > > - Addressed some cosmetic comments from Geert.
> > >
> > > ---
> > > drivers/pmdomain/core.c | 124 ++++++++++++++++++++++++++++++++++++++
> > > include/linux/pm_domain.h | 1 +
> > > 2 files changed, 125 insertions(+)
> > >
> > > diff --git a/drivers/pmdomain/core.c b/drivers/pmdomain/core.c
> > > index ad57846f02a3..c01a9a96e5c2 100644
> > > --- a/drivers/pmdomain/core.c
> > > +++ b/drivers/pmdomain/core.c
> > > @@ -2699,6 +2699,119 @@ static struct generic_pm_domain *genpd_get_from_provider(
> > > return genpd;
> > > }
> > >
> > > +static bool genpd_should_wait_for_consumer(struct device_node *np)
> > > +{
> > > + struct generic_pm_domain *genpd;
> > > + bool should_wait = false;
> > > +
> > > + mutex_lock(&gpd_list_lock);
> > > + list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
> > > + if (genpd->provider == of_fwnode_handle(np)) {
> > > + genpd_lock(genpd);
> > > +
> > > + /* Clear the previous state before reevaluating. */
> > > + genpd->wait_for_consumer = false;
> > > +
> > > + /*
> > > + * Unless there is at least one genpd for the provider
> > > + * that is being kept powered-on, we don't have to care
> > > + * about waiting for consumers.
> > > + */
> > > + if (genpd->stay_on)
> > > + should_wait = true;
> > > +
> > > + genpd_unlock(genpd);
> > > + }
> > > + }
> > > + mutex_unlock(&gpd_list_lock);
> >
> > I think I understand the intent of this function, but haven't dug into
> > genpd code enough to comment on this yet. I'll come back to this
> > later.
> >
> > > +
> > > + return should_wait;
> > > +}
> > > +
> > > +static void genpd_parse_for_consumer(struct device_node *sup,
> > > + struct device_node *con)
> > > +{
> > > + struct generic_pm_domain *genpd;
> > > +
> > > + for (unsigned int i = 0; ; i++) {
> > > + struct of_phandle_args pd_args;
> > > +
> > > + if (of_parse_phandle_with_args(con, "power-domains",
> > > + "#power-domain-cells",
> > > + i, &pd_args))
> > > + break;
> >
> > Why not use a while or a do while() instead of this infinite for loop
> > with a break?
>
> I guess it's a matter of personal preference. I'm not sure the code
> gets any nicer with a do/while, but if you really insist I can change
> it.
>
> >
> > > +
> > > + /*
> > > + * The phandle must correspond to the supplier's genpd provider
> > > + * to be relevant else let's move to the next index.
> > > + */
> > > + if (sup != pd_args.np) {
> > > + of_node_put(pd_args.np);
> > > + continue;
> > > + }
> > > +
> > > + mutex_lock(&gpd_list_lock);
> > > + genpd = genpd_get_from_provider(&pd_args);
> > > + if (!IS_ERR(genpd)) {
> > > + genpd_lock(genpd);
> > > + genpd->wait_for_consumer = true;
> > > + genpd_unlock(genpd);
> > > + }
> > > + mutex_unlock(&gpd_list_lock);
> > > +
> > > + of_node_put(pd_args.np);
> > > + }
> > > +}
> > > +
> > > +static void _genpd_queue_sync_state(struct device_node *np)
> > > +{
> > > + struct generic_pm_domain *genpd;
> > > +
> > > + mutex_lock(&gpd_list_lock);
> > > + list_for_each_entry(genpd, &gpd_list, gpd_list_node) {
> > > + if (genpd->provider == of_fwnode_handle(np)) {
> > > + genpd_lock(genpd);
> > > + if (genpd->stay_on && !genpd->wait_for_consumer) {
> > > + genpd->stay_on = false;
> > > + genpd_queue_power_off_work(genpd);
> > > + }
> > > + genpd_unlock(genpd);
> > > + }
> > > + }
> > > + mutex_unlock(&gpd_list_lock);
> > > +}
> > > +
> > > +static void genpd_queue_sync_state(struct device *dev)
> > > +{
> > > + struct device_node *np = dev->of_node;
> > > + struct device_link *link;
> > > +
> > > + if (!genpd_should_wait_for_consumer(np))
> > > + return;
> > > +
> > > + list_for_each_entry(link, &dev->links.consumers, s_node) {
> >
> > Couple of issues:
> > 1. I don't want the frameworks to be so deeply aware of driver core
> > internals. I want the driver core maintainers to be able to change the
> > devlink implementation without having to worry about going and fixing
> > all the frameworks. So, please add a for_each_consumer_dev(supplier,
> > callback) and for_each_supplier_dev(consumer, callback) helper
> > functions.
>
> I understand your concern and I like the idea. However, maybe it's
> better to get this landed (the series is complicated as is) first and
> then can continue to improve the code on top, with helper functions
> etc.
That patch should be pretty simple, so let's just do it? You are
literally just moving the code to another file and massaging it a bit.
I can send one out if you want/
> >
> > 2. This doesn't ignore "SYNC_STATE_ONLY" links and that's going to
> > confuse the consumer count/check you might do or at the least waste
> > parsing those.
>
> I am not sure I understand how I should take SYNC_STATE_ONLY links
> into account here.
>
> At each call to the genpd_queue_sync_state(), we walk through all the
> provided genpds for the provider. No previous state is cached to track
> consumer counts.
Ok, correct me if I'm wrong here, but it looks like you are looking at
all the consumers, finding the ones that haven't probed yet and then
trying to see which specific genpd provider each one of them is using
by parsing their dt properties and then keep those ON.
If my understanding above is wrong then, please help me understand
what the code is trying to do.
If it's right, then here's the issue:
fw_devlink relies on device links and fwnode links to keep track of
consumers. fwnode links get created first and then get converted to
device links when both the supplier AND consumer devices are created.
Consider a case when the supplier S is created and the consumer device
C is several levels deep inside a parent device A.
S { #power-domain-cells = <1>; }
A { B { C { power-domains = <&S MY_DOMAIN>; } } }
fw_devlink can't just go "oh there's no consumer device that hasn't
probed yet, let me call sync_state()". It needs to wait for C. But
there's no way to create a device link to C. So, here's the sequence
that happens:
1. When device A gets added, it creates a "proxy" SYNC_STATE_ONLY link
between S and A.
2. When A probes, it adds device B.
3. The SYNC_STATE_ONLY link between S to B is created.
4. The SYNC_STATE_ONLY link between S to A is deleted.
5. When B probes, it adds device C.
6. The actual device link between S and C is created.
7. The SYNC_STATE_ONLY link between S to B is deleted.
8. Device C probes, the device links get updated, sync_state() gets called.
So, if your code doesn't account for SYNC_STATE_ONLY links, you are
going to check device "A" to see which providers are used. You'll
think that MY_DOMAIN isn't used by any unprobed consumer and turn it
off.
>
> >
> > 3. **Device** links are not the complete list of consumers because
> > they can only link consumer **devices** once the consumer **device**
> > is created.
> >
> > 4. What you really need is a for_each_consumer_fwnode(supplier,
> > callback) that first loops through all the consumer device links and
> > calls the callback() on their fwnode and then the same function needs
> > to loop through all the fwnode links and then pass those consumer
> > fwnodes to the callback. And inside that callback you can do whatever
> > you want.
>
> The ->queue_sync_state() callback is invoked *after*
> __device_links_queue_sync_state() has been called for the device,
> which is also when the conditions for calling ->sync_state() is
> checked.
I'm not sure what you are trying to say here. Yes, what you are saying
is true. But at the point the current code returns before calling
sync_state(), your patch 2/13 ends up calling the "consumer_probed()"
callback.
If you see the example I gave above, there is a SYNC_STATE_ONLY link
all the way up to step 7 that'll prevent sync_state() from being
called. And until step 5, there is no device link between S and C and
you'll have to go look at S's fwnode links to find C.
> If there are problems with not yet registered consumer device links,
> why isn't that as problem for regular ->sync_state() in
> __device_links_queue_sync_state()?
Does my explanation above make sense?
Welcome to the annoying worlds of fw_devlink corner cases/nuances.
There's a case where the child is the supplier of the
parent/grandparent. There's a comment for that in the fw_devlink code.
Thanks,
Saravana
> >
> > 5. You might want to add a for_each_inactive_consumer(supplier,
> > callback) too to simplify your need for checking if a fwnode has a
> > device and then checking if it's probed.
> >
> > Thanks,
> > Saravana
> >
> >
> > > + struct device *consumer = link->consumer;
> > > +
> > > + if (!device_link_test(link, DL_FLAG_MANAGED))
> > > + continue;
> > > +
> > > + if (link->status == DL_STATE_ACTIVE)
> > > + continue;
> > > +
> > > + if (!consumer->of_node)
> > > + continue;
> > > +
> > > + /*
> > > + * A consumer device has not been probed yet. Let's parse its
> > > + * device node for the power-domains property, to find out the
> > > + * genpds it may belong to and then prevent sync state for them.
> > > + */
> > > + genpd_parse_for_consumer(np, consumer->of_node);
> > > + }
> > > +
> > > + _genpd_queue_sync_state(np);
> > > +}
> > > +
>
> [...]
>
> Kind regards
> Uffe
^ permalink raw reply
* [PATCH RESEND v29 4/4] i2c: ast2600: Add target mode support
From: Ryan Chen @ 2026-05-13 5:32 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen
In-Reply-To: <20260513-upstream_i2c-v29-0-fe9926964d55@aspeedtech.com>
Add target mode support to the AST2600 I2C driver.
Target mode features implemented include:
- Add target interrupt handling
- Address match and response logic
This complements the existing controller-mode support, enabling
dual-role capability.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v29:
- fix race between unreg_target and IRQ handler.
- move i2cs ier enable from ast2600_i2c_init to probe after master ier enable.
- remove dma/byte transfer, use buffer mode only.
Changes in v28:
- fix typo condication -> condition
- fix compile error, when disable CONFIG_I2C_SLAVE
Changes in v26:
- change int to bool target_operate
- rename target_operate to target_active
- use i2c_bus->target replace require IO
- use WRITE_ONCE replace target_operate write.
---
drivers/i2c/busses/i2c-ast2600.c | 340 +++++++++++++++++++++++++++++++++++++++
1 file changed, 340 insertions(+)
diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
index 787ef6bd6244..137aaf7e26b5 100644
--- a/drivers/i2c/busses/i2c-ast2600.c
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -254,6 +254,11 @@ struct ast2600_i2c_bus {
size_t buf_size;
bool multi_master;
void __iomem *buf_base;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ /* target structure */
+ bool target_active;
+ struct i2c_client *target;
+#endif
};
static void ast2600_i2c_ac_timing_config(struct ast2600_i2c_bus *i2c_bus)
@@ -337,6 +342,243 @@ static int ast2600_i2c_recover_bus(struct ast2600_i2c_bus *i2c_bus)
return ret;
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static void ast2600_i2c_target_packet_buff_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+ int target_rx_len = 0;
+ u32 cmd = 0;
+ u8 value;
+ int i;
+
+ /* due to controller target is common buffer, need force the master stop not issue */
+ if (readl(i2c_bus->reg_base + AST2600_I2CM_CMD_STS) & GENMASK(15, 0)) {
+ writel(0, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+ i2c_bus->cmd_err = -EBUSY;
+ writel(0, i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ complete(&i2c_bus->cmd_complete);
+ }
+
+ /* Handle i2c target timeout condition */
+ if (AST2600_I2CS_INACTIVE_TO & sts) {
+ /* Reset timeout counter */
+ u32 ac_timing = readl(i2c_bus->reg_base + AST2600_I2CC_AC_TIMING) &
+ AST2600_I2CC_AC_TIMING_MASK;
+
+ writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+ ac_timing |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+ writel(ac_timing, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+ writel(TARGET_TRIGGER_CMD, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ WRITE_ONCE(i2c_bus->target_active, false);
+ return;
+ }
+
+ sts &= ~(AST2600_I2CS_PKT_DONE | AST2600_I2CS_PKT_ERROR);
+
+ if (sts & AST2600_I2CS_SLAVE_MATCH)
+ WRITE_ONCE(i2c_bus->target_active, true);
+
+ switch (sts) {
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
+ AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+ case AST2600_I2CS_SLAVE_PENDING |
+ AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+ case AST2600_I2CS_SLAVE_PENDING |
+ AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_STOP:
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ fallthrough;
+ case AST2600_I2CS_SLAVE_PENDING |
+ AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+ case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+ case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_SLAVE_MATCH:
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+ cmd = TARGET_TRIGGER_CMD;
+ if (sts & AST2600_I2CS_RX_DONE) {
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ }
+ if (readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS) & AST2600_I2CS_RX_BUFF_EN)
+ cmd = 0;
+ else
+ cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+
+ writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ break;
+ case AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_RX_DONE:
+ cmd = TARGET_TRIGGER_CMD;
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ cmd |= AST2600_I2CS_RX_BUFF_EN;
+ writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ break;
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
+ AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+ cmd = TARGET_TRIGGER_CMD;
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ cmd |= AST2600_I2CS_RX_BUFF_EN;
+ writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ break;
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+ cmd = TARGET_TRIGGER_CMD;
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ /* workaround for avoid next start with len != 0 */
+ writel(BIT(0), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ break;
+ case AST2600_I2CS_RX_DONE | AST2600_I2CS_STOP:
+ cmd = TARGET_TRIGGER_CMD;
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ /* workaround for avoid next start with len != 0 */
+ writel(BIT(0), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ break;
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_RX_DONE |
+ AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_STOP:
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+ writeb(value, i2c_bus->buf_base);
+ break;
+ case AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_SLAVE_MATCH:
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+ writeb(value, i2c_bus->buf_base);
+ writel(AST2600_I2CC_SET_TX_BUF_LEN(1),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_BUFF_EN;
+ break;
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_STOP |
+ AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+ case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA | AST2600_I2CS_STOP |
+ AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ writel(AST2600_I2CC_SET_RX_BUF_LEN(i2c_bus->buf_size),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+ break;
+ case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_RX_DONE:
+ case AST2600_I2CS_WAIT_TX_DMA | AST2600_I2CS_RX_DONE:
+ case AST2600_I2CS_WAIT_TX_DMA:
+ if (sts & AST2600_I2CS_SLAVE_MATCH)
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+
+ if (sts & AST2600_I2CS_RX_DONE) {
+ target_rx_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < target_rx_len; i++) {
+ value = readb(i2c_bus->buf_base + 0x10 + i);
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED, &value);
+ }
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_REQUESTED, &value);
+ } else {
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_READ_PROCESSED, &value);
+ }
+ writeb(value, i2c_bus->buf_base);
+ writel(AST2600_I2CC_SET_TX_BUF_LEN(1),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_TX_BUFF_EN;
+ break;
+ /* workaround : trigger the cmd twice to fix next state keep 1000000 */
+ case AST2600_I2CS_SLAVE_MATCH | AST2600_I2CS_RX_DONE:
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED, &value);
+ cmd = TARGET_TRIGGER_CMD | AST2600_I2CS_RX_BUFF_EN;
+ writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ break;
+ case AST2600_I2CS_TX_NAK | AST2600_I2CS_STOP:
+ case AST2600_I2CS_STOP:
+ cmd = TARGET_TRIGGER_CMD;
+ i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
+ break;
+ default:
+ dev_dbg(i2c_bus->dev, "unhandled target isr case %x, sts %x\n", sts,
+ readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
+ break;
+ }
+
+ if (cmd)
+ writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_ISR);
+ readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+ if ((sts & AST2600_I2CS_STOP) && !(sts & AST2600_I2CS_SLAVE_PENDING))
+ WRITE_ONCE(i2c_bus->target_active, false);
+}
+
+static int ast2600_i2c_target_irq(struct ast2600_i2c_bus *i2c_bus)
+{
+ u32 ier = readl(i2c_bus->reg_base + AST2600_I2CS_IER);
+ u32 isr = readl(i2c_bus->reg_base + AST2600_I2CS_ISR);
+
+ if (!(isr & ier))
+ return 0;
+
+ /*
+ * Target interrupt coming after controller packet done
+ * So need handle controller first.
+ */
+ if (readl(i2c_bus->reg_base + AST2600_I2CM_ISR) & AST2600_I2CM_PKT_DONE)
+ return 0;
+
+ isr &= ~(AST2600_I2CS_ADDR_INDICATE_MASK);
+
+ if (AST2600_I2CS_ADDR1_NAK & isr)
+ isr &= ~AST2600_I2CS_ADDR1_NAK;
+
+ if (AST2600_I2CS_ADDR2_NAK & isr)
+ isr &= ~AST2600_I2CS_ADDR2_NAK;
+
+ if (AST2600_I2CS_ADDR3_NAK & isr)
+ isr &= ~AST2600_I2CS_ADDR3_NAK;
+
+ if (AST2600_I2CS_ADDR_MASK & isr)
+ isr &= ~AST2600_I2CS_ADDR_MASK;
+
+ if (AST2600_I2CS_PKT_DONE & isr)
+ ast2600_i2c_target_packet_buff_irq(i2c_bus, isr);
+
+ return 1;
+}
+#endif
+
static int ast2600_i2c_setup_buff_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
{
struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
@@ -498,6 +740,20 @@ static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u
}
break;
case AST2600_I2CM_RX_DONE:
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ /*
+ * Workaround for controller/target packet mode enable rx done stuck issue
+ * When controller go for first read (RX_DONE), target mode will also effect
+ * Then controller will send nack, not operate anymore.
+ */
+ if (readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS) & AST2600_I2CS_PKT_MODE_EN) {
+ u32 target_cmd = readl(i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+
+ writel(0, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ writel(target_cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ }
+ fallthrough;
+#endif
case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
/* do next rx */
xfer_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
@@ -584,6 +840,12 @@ static irqreturn_t ast2600_i2c_bus_irq(int irq, void *dev_id)
{
struct ast2600_i2c_bus *i2c_bus = dev_id;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (i2c_bus->target) {
+ if (ast2600_i2c_target_irq(i2c_bus))
+ return IRQ_HANDLED;
+ }
+#endif
return IRQ_RETVAL(ast2600_i2c_controller_irq(i2c_bus));
}
@@ -600,12 +862,31 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
return ret;
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (i2c_bus->target_active)
+ return -EBUSY;
+ /*
+ * Controller and target share the same buffer register. A target
+ * transaction can update buffer state asynchronously via IRQ, so block
+ * controller transfers while target is active to avoid buffer corruption.
+ */
+ writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);
+ if (readl(i2c_bus->reg_base + AST2600_I2CS_ISR) || i2c_bus->target_active) {
+ writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+ return -EBUSY;
+ }
+#endif
+
i2c_bus->cmd_err = 0;
i2c_bus->msgs = msgs;
i2c_bus->msgs_index = 0;
i2c_bus->msgs_count = num;
reinit_completion(&i2c_bus->cmd_complete);
ret = ast2600_i2c_do_start(i2c_bus);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ /* avoid race condition target is wait and controller wait 1st target operate */
+ writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+#endif
if (ret)
goto controller_out;
timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
@@ -624,6 +905,9 @@ static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg
* if the bus is still busy.
*/
if (i2c_bus->multi_master &&
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ !i2c_bus->target_active &&
+#endif
(readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
AST2600_I2CC_BUS_BUSY_STS))
ast2600_i2c_recover_bus(i2c_bus);
@@ -659,8 +943,54 @@ static int ast2600_i2c_init(struct ast2600_i2c_bus *i2c_bus)
/* Clear Interrupt */
writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CM_ISR);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CS_ISR);
+#endif
+
+ return 0;
+}
+
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static int ast2600_i2c_reg_target(struct i2c_client *client)
+{
+ struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(client->adapter);
+ u32 cmd = TARGET_TRIGGER_CMD;
+
+ if (i2c_bus->target)
+ return -EINVAL;
+
+ dev_dbg(i2c_bus->dev, "target addr %x\n", client->addr);
+
+ writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+ writel(AST2600_I2CC_SLAVE_EN | readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL),
+ i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+ writel(cmd, i2c_bus->reg_base + AST2600_I2CS_CMD_STS);
+ i2c_bus->target = client;
+ /* Set target addr. */
+ writel(client->addr | AST2600_I2CS_ADDR1_ENABLE,
+ i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+ return 0;
+}
+
+static int ast2600_i2c_unreg_target(struct i2c_client *client)
+{
+ struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(client->adapter);
+ u32 val;
+
+ /* Turn off target mode. */
+ val = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(val & ~AST2600_I2CC_SLAVE_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ val = readl(i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+ writel(val & ~AST2600_I2CS_ADDR1_MASK, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+ synchronize_irq(i2c_bus->irq);
+ i2c_bus->target = NULL;
+
return 0;
}
+#endif
static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
{
@@ -670,6 +1000,10 @@ static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
static const struct i2c_algorithm i2c_ast2600_algorithm = {
.xfer = ast2600_i2c_controller_xfer,
.functionality = ast2600_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ .reg_target = ast2600_i2c_reg_target,
+ .unreg_target = ast2600_i2c_unreg_target,
+#endif
};
static int ast2600_i2c_probe(struct platform_device *pdev)
@@ -708,6 +1042,9 @@ static int ast2600_i2c_probe(struct platform_device *pdev)
regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ WRITE_ONCE(i2c_bus->target_active, false);
+#endif
i2c_bus->dev = dev;
i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
@@ -765,6 +1102,9 @@ static int ast2600_i2c_probe(struct platform_device *pdev)
writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
i2c_bus->reg_base + AST2600_I2CM_IER);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base + AST2600_I2CS_IER);
+#endif
ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
if (ret)
--
2.34.1
^ permalink raw reply related
* [PATCH RESEND v29 3/4] i2c: ast2600: Add controller driver for AST2600 new register set
From: Ryan Chen @ 2026-05-13 5:32 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen
In-Reply-To: <20260513-upstream_i2c-v29-0-fe9926964d55@aspeedtech.com>
The AST2600 introduces a new I2C controller register layout, selectable
at runtime via global control registers. Compared to the legacy layout
used on AST2400/AST2500, the new layout separates controller (master)
and target (slave) registers and adds support for packet-based transfers
The new register set extends the hardware capabilities with:
- Enhanced clock divider configuration for improved timing precision
- tCKHighMin timing control for SCL high pulse width
- Dual pool buffer mode (separate Tx/Rx buffers)
- Hardware-assisted bus recovery and timeout mechanisms
This patch adds an AST2600-specific I2C controller driver implementing
the new register layout, including support for packet-based transfers.
The legacy and new register layouts represent the same AST2600 I2C
controller IP and therefore share the existing compatible string:
"aspeed,ast2600-i2c-bus"
To preserve DT ABI compatibility, driver selection is performed at probe
time based on DT contents. In particular, the new binding requires the
`aspeed,global-regs` phandle, which is absent from legacy DTBs:
- The new driver only probes successfully when `aspeed,global-regs` is
present.
- The existing i2c-aspeed driver returns -ENODEV for AST2600 nodes that
provide `aspeed,global-regs`, allowing the new driver to bind.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v29:
- update commit message remove transfer mode selection.
- remove dma/byte transfer, use buffer mode only.
- remove sysfs file.
- remove define I2C_TARGET_MSG_BUF_SIZE and AST2600_I2C_DMA_SIZE.
- remove buf_index in struct ast2600_i2c_bus.
Changes in v28:
- Separate xfer_mode_store into distinct parse and availability-check
steps by introducing ast2600_i2c_xfer_mode_check()
- fix tx dma memcpy source point address.
- Use a temporary variable for devm_platform_get_and_ioremap_resource()
to avoid storing an ERR_PTR in i2c_bus->buf_base; drop the redundant
NULL assignment in the error path since i2c_bus is kzalloc()ed
- Add ABI documentation file
Documentation/ABI/testing/sysfs-driver-ast2600-i2c
Changes in v27:
- remove aspeed,transfer-mode selection instead aspeed,dma-mode.
- add sysfs for xfer mode.
Changes in v25:
- Rename AST2600_I2CM_SMBUS_ALT to AST2600_I2CM_SMBUS_ALERT.
- Refactor transfer mode handling using setup_tx/setup_rx helpers.
- Rework DMA handling to use pre-allocated buffers and reduce
mapping overhead in interrupt context.
- Fix IRQ status checks to use consistent (sts & value) style.
- Move device_property_read_bool() to probe().
- Improve probe error handling.
- Handle timeout condition in target_byte_irq().
- Rename "package" to "packet".
- Remove target reset when master wait_for_completion_timeout().
---
drivers/i2c/busses/Makefile | 2 +-
drivers/i2c/busses/i2c-aspeed.c | 5 +
drivers/i2c/busses/i2c-ast2600.c | 808 +++++++++++++++++++++++++++++++++++++++
3 files changed, 814 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 547123ab351f..ece201a67d41 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
obj-$(CONFIG_I2C_ALTERA) += i2c-altera.o
obj-$(CONFIG_I2C_AMD_MP2) += i2c-amd-mp2-pci.o i2c-amd-mp2-plat.o
obj-$(CONFIG_I2C_AMD_ASF) += i2c-amd-asf-plat.o
-obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
+obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o i2c-ast2600.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
i2c-at91-y := i2c-at91-core.o i2c-at91-master.o
i2c-at91-$(CONFIG_I2C_AT91_SLAVE_EXPERIMENTAL) += i2c-at91-slave.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a26b74c71206..8286fd2cd130 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -22,6 +22,7 @@
#include <linux/of_irq.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/reset.h>
#include <linux/slab.h>
@@ -1002,6 +1003,10 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
struct clk *parent_clk;
int irq, ret;
+ if (device_is_compatible(&pdev->dev, "aspeed,ast2600-i2c-bus") &&
+ device_property_present(&pdev->dev, "aspeed,global-regs"))
+ return -ENODEV;
+
bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
if (!bus)
return -ENOMEM;
diff --git a/drivers/i2c/busses/i2c-ast2600.c b/drivers/i2c/busses/i2c-ast2600.c
new file mode 100644
index 000000000000..787ef6bd6244
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ast2600.c
@@ -0,0 +1,808 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ASPEED AST2600 new register set I2C controller driver
+ *
+ * Copyright (C) 2026 ASPEED Technology Inc.
+ */
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/minmax.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+#include <linux/unaligned.h>
+
+#define AST2600_I2CG_ISR 0x00
+#define AST2600_I2CG_SLAVE_ISR 0x04
+#define AST2600_I2CG_OWNER 0x08
+#define AST2600_I2CG_CTRL 0x0C
+#define AST2600_I2CG_CLK_DIV_CTRL 0x10
+
+#define AST2600_I2CG_SLAVE_PKT_NAK BIT(4)
+#define AST2600_I2CG_M_S_SEPARATE_INTR BIT(3)
+#define AST2600_I2CG_CTRL_NEW_REG BIT(2)
+#define AST2600_I2CG_CTRL_NEW_CLK_DIV BIT(1)
+#define AST2600_GLOBAL_INIT \
+ (AST2600_I2CG_CTRL_NEW_REG | AST2600_I2CG_CTRL_NEW_CLK_DIV)
+/*
+ * APB clk : 100Mhz
+ * div : scl : baseclk [APB/((div/2) + 1)] : tBuf [1/bclk * 16]
+ * I2CG10[31:24] base clk4 for i2c auto recovery timeout counter (0xC6)
+ * I2CG10[23:16] base clk3 for Standard-mode (100Khz) min tBuf 4.7us
+ * 0x3c : 100.8Khz : 3.225Mhz : 4.96us
+ * 0x3d : 99.2Khz : 3.174Mhz : 5.04us
+ * 0x3e : 97.65Khz : 3.125Mhz : 5.12us
+ * 0x40 : 97.75Khz : 3.03Mhz : 5.28us
+ * 0x41 : 99.5Khz : 2.98Mhz : 5.36us (default)
+ * I2CG10[15:8] base clk2 for Fast-mode (400Khz) min tBuf 1.3us
+ * 0x12 : 400Khz : 10Mhz : 1.6us
+ * I2CG10[7:0] base clk1 for Fast-mode Plus (1Mhz) min tBuf 0.5us
+ * 0x08 : 1Mhz : 20Mhz : 0.8us
+ */
+#define I2CCG_DIV_CTRL 0xC6411208
+
+/* 0x00 : I2CC Controller/Target Function Control Register */
+#define AST2600_I2CC_FUN_CTRL 0x00
+#define AST2600_I2CC_SLAVE_ADDR_RX_EN BIT(20)
+#define AST2600_I2CC_MASTER_RETRY_MASK GENMASK(19, 18)
+#define AST2600_I2CC_MASTER_RETRY(x) (((x) & GENMASK(1, 0)) << 18)
+#define AST2600_I2CC_BUS_AUTO_RELEASE BIT(17)
+#define AST2600_I2CC_M_SDA_LOCK_EN BIT(16)
+#define AST2600_I2CC_MULTI_MASTER_DIS BIT(15)
+#define AST2600_I2CC_M_SCL_DRIVE_EN BIT(14)
+#define AST2600_I2CC_MSB_STS BIT(9)
+#define AST2600_I2CC_SDA_DRIVE_1T_EN BIT(8)
+#define AST2600_I2CC_M_SDA_DRIVE_1T_EN BIT(7)
+#define AST2600_I2CC_M_HIGH_SPEED_EN BIT(6)
+/* reserver 5 : 2 */
+#define AST2600_I2CC_SLAVE_EN BIT(1)
+#define AST2600_I2CC_MASTER_EN BIT(0)
+
+/* 0x04 : I2CC Controller/Target Clock and AC Timing Control Register #1 */
+#define AST2600_I2CC_AC_TIMING 0x04
+#define AST2600_I2CC_TTIMEOUT(x) (((x) & GENMASK(4, 0)) << 24)
+#define AST2600_I2CC_TCKHIGHMIN(x) (((x) & GENMASK(3, 0)) << 20)
+#define AST2600_I2CC_TCKHIGH(x) (((x) & GENMASK(3, 0)) << 16)
+#define AST2600_I2CC_TCKLOW(x) (((x) & GENMASK(3, 0)) << 12)
+#define AST2600_I2CC_THDDAT(x) (((x) & GENMASK(1, 0)) << 10)
+#define AST2600_I2CC_TOUTBASECLK(x) (((x) & GENMASK(1, 0)) << 8)
+#define AST2600_I2CC_TBASECLK(x) ((x) & GENMASK(3, 0))
+#define AST2600_I2CC_AC_TIMING_MASK GENMASK(23, 0)
+
+/* 0x08 : I2CC Controller/Target Transmit/Receive Byte Buffer Register */
+#define AST2600_I2CC_STS_AND_BUFF 0x08
+#define AST2600_I2CC_TX_DIR_MASK GENMASK(31, 29)
+#define AST2600_I2CC_SDA_OE BIT(28)
+#define AST2600_I2CC_SDA_O BIT(27)
+#define AST2600_I2CC_SCL_OE BIT(26)
+#define AST2600_I2CC_SCL_O BIT(25)
+
+#define AST2600_I2CC_SCL_LINE_STS BIT(18)
+#define AST2600_I2CC_SDA_LINE_STS BIT(17)
+#define AST2600_I2CC_BUS_BUSY_STS BIT(16)
+
+#define AST2600_I2CC_GET_RX_BUFF(x) (((x) >> 8) & GENMASK(7, 0))
+
+/* 0x0C : I2CC Controller/Target Pool Buffer Control Register */
+#define AST2600_I2CC_BUFF_CTRL 0x0C
+#define AST2600_I2CC_GET_RX_BUF_LEN(x) (((x) & GENMASK(29, 24)) >> 24)
+#define AST2600_I2CC_SET_RX_BUF_LEN(x) (((((x) - 1) & GENMASK(4, 0)) << 16) | BIT(0))
+#define AST2600_I2CC_SET_TX_BUF_LEN(x) (((((x) - 1) & GENMASK(4, 0)) << 8) | BIT(0))
+#define AST2600_I2CC_GET_TX_BUF_LEN(x) ((((x) & GENMASK(12, 8)) >> 8) + 1)
+
+/* 0x10 : I2CM Controller Interrupt Control Register */
+#define AST2600_I2CM_IER 0x10
+/* 0x14 : I2CM Controller Interrupt Status Register : WC */
+#define AST2600_I2CM_ISR 0x14
+
+#define AST2600_I2CM_PKT_TIMEOUT BIT(18)
+#define AST2600_I2CM_PKT_ERROR BIT(17)
+#define AST2600_I2CM_PKT_DONE BIT(16)
+
+#define AST2600_I2CM_BUS_RECOVER_FAIL BIT(15)
+#define AST2600_I2CM_SDA_DL_TO BIT(14)
+#define AST2600_I2CM_BUS_RECOVER BIT(13)
+#define AST2600_I2CM_SMBUS_ALERT BIT(12)
+
+#define AST2600_I2CM_SCL_LOW_TO BIT(6)
+#define AST2600_I2CM_ABNORMAL BIT(5)
+#define AST2600_I2CM_NORMAL_STOP BIT(4)
+#define AST2600_I2CM_ARBIT_LOSS BIT(3)
+#define AST2600_I2CM_RX_DONE BIT(2)
+#define AST2600_I2CM_TX_NAK BIT(1)
+#define AST2600_I2CM_TX_ACK BIT(0)
+
+/* 0x18 : I2CM Controller Command/Status Register */
+#define AST2600_I2CM_CMD_STS 0x18
+#define AST2600_I2CM_PKT_ADDR(x) (((x) & GENMASK(6, 0)) << 24)
+#define AST2600_I2CM_PKT_EN BIT(16)
+#define AST2600_I2CM_SDA_OE_OUT_DIR BIT(15)
+#define AST2600_I2CM_SDA_O_OUT_DIR BIT(14)
+#define AST2600_I2CM_SCL_OE_OUT_DIR BIT(13)
+#define AST2600_I2CM_SCL_O_OUT_DIR BIT(12)
+#define AST2600_I2CM_RECOVER_CMD_EN BIT(11)
+
+#define AST2600_I2CM_RX_DMA_EN BIT(9)
+#define AST2600_I2CM_TX_DMA_EN BIT(8)
+/* Command Bit */
+#define AST2600_I2CM_RX_BUFF_EN BIT(7)
+#define AST2600_I2CM_TX_BUFF_EN BIT(6)
+#define AST2600_I2CM_STOP_CMD BIT(5)
+#define AST2600_I2CM_RX_CMD_LAST BIT(4)
+#define AST2600_I2CM_RX_CMD BIT(3)
+
+#define AST2600_I2CM_TX_CMD BIT(1)
+#define AST2600_I2CM_START_CMD BIT(0)
+
+/* 0x1C : I2CM Controller DMA Transfer Length Register */
+#define AST2600_I2CM_DMA_LEN 0x1C
+/* Tx Rx support length 1 ~ 4096 */
+#define AST2600_I2CM_SET_RX_DMA_LEN(x) ((((x) & GENMASK(11, 0)) << 16) | BIT(31))
+#define AST2600_I2CM_SET_TX_DMA_LEN(x) (((x) & GENMASK(11, 0)) | BIT(15))
+
+/* 0x20 : I2CS Target Interrupt Control Register */
+#define AST2600_I2CS_IER 0x20
+/* 0x24 : I2CS Target Interrupt Status Register */
+#define AST2600_I2CS_ISR 0x24
+
+#define AST2600_I2CS_ADDR_INDICATE_MASK GENMASK(31, 30)
+#define AST2600_I2CS_SLAVE_PENDING BIT(29)
+
+#define AST2600_I2CS_WAIT_TX_DMA BIT(25)
+#define AST2600_I2CS_WAIT_RX_DMA BIT(24)
+
+#define AST2600_I2CS_ADDR3_NAK BIT(22)
+#define AST2600_I2CS_ADDR2_NAK BIT(21)
+#define AST2600_I2CS_ADDR1_NAK BIT(20)
+
+#define AST2600_I2CS_ADDR_MASK GENMASK(19, 18)
+#define AST2600_I2CS_PKT_ERROR BIT(17)
+#define AST2600_I2CS_PKT_DONE BIT(16)
+#define AST2600_I2CS_INACTIVE_TO BIT(15)
+
+#define AST2600_I2CS_SLAVE_MATCH BIT(7)
+#define AST2600_I2CS_ABNOR_STOP BIT(5)
+#define AST2600_I2CS_STOP BIT(4)
+#define AST2600_I2CS_RX_DONE_NAK BIT(3)
+#define AST2600_I2CS_RX_DONE BIT(2)
+#define AST2600_I2CS_TX_NAK BIT(1)
+#define AST2600_I2CS_TX_ACK BIT(0)
+
+/* 0x28 : I2CS Target CMD/Status Register */
+#define AST2600_I2CS_CMD_STS 0x28
+#define AST2600_I2CS_ACTIVE_ALL GENMASK(18, 17)
+#define AST2600_I2CS_PKT_MODE_EN BIT(16)
+#define AST2600_I2CS_AUTO_NAK_NOADDR BIT(15)
+#define AST2600_I2CS_AUTO_NAK_EN BIT(14)
+
+#define AST2600_I2CS_ALT_EN BIT(10)
+#define AST2600_I2CS_RX_DMA_EN BIT(9)
+#define AST2600_I2CS_TX_DMA_EN BIT(8)
+#define AST2600_I2CS_RX_BUFF_EN BIT(7)
+#define AST2600_I2CS_TX_BUFF_EN BIT(6)
+#define AST2600_I2CS_RX_CMD_LAST BIT(4)
+
+#define AST2600_I2CS_TX_CMD BIT(2)
+
+#define AST2600_I2CS_DMA_LEN 0x2C
+#define AST2600_I2CS_SET_RX_DMA_LEN(x) (((((x) - 1) & GENMASK(11, 0)) << 16) | BIT(31))
+#define AST2600_I2CS_SET_TX_DMA_LEN(x) ((((x) - 1) & GENMASK(11, 0)) | BIT(15))
+
+/* I2CM Controller DMA Tx Buffer Register */
+#define AST2600_I2CM_TX_DMA 0x30
+/* I2CM Controller DMA Rx Buffer Register */
+#define AST2600_I2CM_RX_DMA 0x34
+/* I2CS Target DMA Tx Buffer Register */
+#define AST2600_I2CS_TX_DMA 0x38
+/* I2CS Target DMA Rx Buffer Register */
+#define AST2600_I2CS_RX_DMA 0x3C
+
+#define AST2600_I2CS_ADDR_CTRL 0x40
+
+#define AST2600_I2CS_ADDR3_MASK GENMASK(22, 16)
+#define AST2600_I2CS_ADDR2_MASK GENMASK(14, 8)
+#define AST2600_I2CS_ADDR1_MASK GENMASK(6, 0)
+
+#define AST2600_I2CM_DMA_LEN_STS 0x48
+#define AST2600_I2CS_DMA_LEN_STS 0x4C
+
+#define AST2600_I2C_GET_TX_DMA_LEN(x) ((x) & GENMASK(12, 0))
+#define AST2600_I2C_GET_RX_DMA_LEN(x) (((x) & GENMASK(28, 16)) >> 16)
+
+/* 0x40 : Target Device Address Register */
+#define AST2600_I2CS_ADDR3_ENABLE BIT(23)
+#define AST2600_I2CS_ADDR3(x) ((x) << 16)
+#define AST2600_I2CS_ADDR2_ENABLE BIT(15)
+#define AST2600_I2CS_ADDR2(x) ((x) << 8)
+#define AST2600_I2CS_ADDR1_ENABLE BIT(7)
+#define AST2600_I2CS_ADDR1(x) (x)
+
+#define CONTROLLER_TRIGGER_LAST_STOP (AST2600_I2CM_RX_CMD_LAST | AST2600_I2CM_STOP_CMD)
+#define TARGET_TRIGGER_CMD (AST2600_I2CS_ACTIVE_ALL | AST2600_I2CS_PKT_MODE_EN)
+
+#define AST_I2C_TIMEOUT_CLK 0x1
+
+struct ast2600_i2c_bus {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem *reg_base;
+ struct regmap *global_regs;
+ struct clk *clk;
+ struct i2c_timings timing_info;
+ struct completion cmd_complete;
+ struct i2c_msg *msgs;
+ u32 apb_clk;
+ u32 timeout;
+ int irq;
+ int cmd_err;
+ int msgs_index;
+ int msgs_count;
+ int controller_xfer_cnt;
+ size_t buf_size;
+ bool multi_master;
+ void __iomem *buf_base;
+};
+
+static void ast2600_i2c_ac_timing_config(struct ast2600_i2c_bus *i2c_bus)
+{
+ unsigned long base_clk[16];
+ int baseclk_idx = 0;
+ int divisor = 0;
+ u32 clk_div_reg;
+ u32 scl_low;
+ u32 scl_high;
+ u32 data;
+
+ regmap_read(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, &clk_div_reg);
+
+ for (int i = 0; i < ARRAY_SIZE(base_clk); i++) {
+ if (i == 0)
+ base_clk[i] = i2c_bus->apb_clk;
+ else if (i < 5)
+ base_clk[i] = (i2c_bus->apb_clk * 2) /
+ (((clk_div_reg >> ((i - 1) * 8)) & GENMASK(7, 0)) + 2);
+ else
+ base_clk[i] = base_clk[4] >> (i - 4);
+
+ if ((base_clk[i] / i2c_bus->timing_info.bus_freq_hz) <= 32) {
+ baseclk_idx = i;
+ divisor = DIV_ROUND_UP(base_clk[i], i2c_bus->timing_info.bus_freq_hz);
+ break;
+ }
+ }
+ baseclk_idx = min(baseclk_idx, 15);
+ divisor = min(divisor, 32);
+ scl_low = min(divisor * 9 / 16 - 1, 15);
+ scl_high = (divisor - scl_low - 2) & GENMASK(3, 0);
+ data = (scl_high - 1) << 20 | scl_high << 16 | scl_low << 12 | baseclk_idx;
+ if (i2c_bus->timeout) {
+ data |= AST2600_I2CC_TOUTBASECLK(AST_I2C_TIMEOUT_CLK);
+ data |= AST2600_I2CC_TTIMEOUT(i2c_bus->timeout);
+ }
+
+ writel(data, i2c_bus->reg_base + AST2600_I2CC_AC_TIMING);
+}
+
+static int ast2600_i2c_recover_bus(struct ast2600_i2c_bus *i2c_bus)
+{
+ u32 state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+ int ret = 0;
+ u32 ctrl;
+ int r;
+
+ dev_dbg(i2c_bus->dev, "%d-bus recovery bus [%x]\n", i2c_bus->adap.nr, state);
+
+ /* reset controller */
+ ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+ reinit_completion(&i2c_bus->cmd_complete);
+ i2c_bus->cmd_err = 0;
+
+ /* Check SDA/SCL status in the status register. */
+ state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+ if (!(state & AST2600_I2CC_SDA_LINE_STS) && (state & AST2600_I2CC_SCL_LINE_STS)) {
+ writel(AST2600_I2CM_RECOVER_CMD_EN, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+ r = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
+ if (r == 0) {
+ dev_dbg(i2c_bus->dev, "recovery timed out\n");
+ return -ETIMEDOUT;
+ } else if (i2c_bus->cmd_err) {
+ dev_dbg(i2c_bus->dev, "recovery error\n");
+ ret = -EPROTO;
+ }
+ }
+
+ /* Recovery done */
+ state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
+ if (state & AST2600_I2CC_BUS_BUSY_STS) {
+ dev_dbg(i2c_bus->dev, "Can't recover bus [%x]\n", state);
+ ret = -EPROTO;
+ }
+
+ return ret;
+}
+
+static int ast2600_i2c_setup_buff_tx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+ struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+ int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+ u32 wbuf_dword;
+ int i;
+
+ cmd |= AST2600_I2CM_PKT_EN;
+
+ if (xfer_len > i2c_bus->buf_size)
+ xfer_len = i2c_bus->buf_size;
+ else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count)
+ cmd |= AST2600_I2CM_STOP_CMD;
+
+ if (cmd & AST2600_I2CM_START_CMD)
+ cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+ if (xfer_len) {
+ cmd |= AST2600_I2CM_TX_BUFF_EN | AST2600_I2CM_TX_CMD;
+ /*
+ * The controller's buffer register supports dword writes only.
+ * Therefore, write dwords to the buffer register in a 4-byte aligned,
+ * and write the remaining unaligned data at the end.
+ */
+ for (i = 0; i < xfer_len; i += 4) {
+ int xfer_cnt = i2c_bus->controller_xfer_cnt + i;
+
+ switch (min(xfer_len - i, 4) % 4) {
+ case 1:
+ wbuf_dword = msg->buf[xfer_cnt];
+ break;
+ case 2:
+ wbuf_dword = get_unaligned_le16(&msg->buf[xfer_cnt]);
+ break;
+ case 3:
+ wbuf_dword = get_unaligned_le24(&msg->buf[xfer_cnt]);
+ break;
+ default:
+ wbuf_dword = get_unaligned_le32(&msg->buf[xfer_cnt]);
+ break;
+ }
+ writel(wbuf_dword, i2c_bus->buf_base + i);
+ }
+ writel(AST2600_I2CC_SET_TX_BUF_LEN(xfer_len),
+ i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+ }
+
+ writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+ return 0;
+}
+
+static int ast2600_i2c_setup_buff_rx(u32 cmd, struct ast2600_i2c_bus *i2c_bus)
+{
+ struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+ int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
+
+ cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_BUFF_EN | AST2600_I2CM_RX_CMD;
+
+ if (cmd & AST2600_I2CM_START_CMD)
+ cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
+
+ if (msg->flags & I2C_M_RECV_LEN) {
+ dev_dbg(i2c_bus->dev, "smbus read\n");
+ xfer_len = 1;
+ } else if (xfer_len > i2c_bus->buf_size) {
+ xfer_len = i2c_bus->buf_size;
+ } else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) {
+ cmd |= CONTROLLER_TRIGGER_LAST_STOP;
+ }
+ writel(AST2600_I2CC_SET_RX_BUF_LEN(xfer_len), i2c_bus->reg_base + AST2600_I2CC_BUFF_CTRL);
+
+ writel(cmd, i2c_bus->reg_base + AST2600_I2CM_CMD_STS);
+
+ return 0;
+}
+
+static int ast2600_i2c_do_start(struct ast2600_i2c_bus *i2c_bus)
+{
+ struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+
+ /* send start */
+ dev_dbg(i2c_bus->dev, "[%d] %s %d byte%s %s 0x%02x\n",
+ i2c_bus->msgs_index, str_read_write(msg->flags & I2C_M_RD),
+ msg->len, str_plural(msg->len),
+ msg->flags & I2C_M_RD ? "from" : "to", msg->addr);
+
+ i2c_bus->controller_xfer_cnt = 0;
+
+ if (msg->flags & I2C_M_RD)
+ return ast2600_i2c_setup_buff_rx(AST2600_I2CM_START_CMD, i2c_bus);
+
+ return ast2600_i2c_setup_buff_tx(AST2600_I2CM_START_CMD, i2c_bus);
+}
+
+static int ast2600_i2c_irq_err_to_errno(u32 irq_status)
+{
+ if (irq_status & AST2600_I2CM_ARBIT_LOSS)
+ return -EAGAIN;
+ if (irq_status & (AST2600_I2CM_SDA_DL_TO | AST2600_I2CM_SCL_LOW_TO))
+ return -ETIMEDOUT;
+ if (irq_status & (AST2600_I2CM_ABNORMAL))
+ return -EPROTO;
+
+ return 0;
+}
+
+static void ast2600_i2c_controller_packet_irq(struct ast2600_i2c_bus *i2c_bus, u32 sts)
+{
+ struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
+ int xfer_len;
+ int i;
+
+ sts &= ~AST2600_I2CM_PKT_DONE;
+ writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base + AST2600_I2CM_ISR);
+ switch (sts) {
+ case AST2600_I2CM_PKT_ERROR:
+ i2c_bus->cmd_err = -EAGAIN;
+ complete(&i2c_bus->cmd_complete);
+ break;
+ case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK: /* a0 fix for issue */
+ fallthrough;
+ case AST2600_I2CM_PKT_ERROR | AST2600_I2CM_TX_NAK | AST2600_I2CM_NORMAL_STOP:
+ i2c_bus->cmd_err = -ENXIO;
+ complete(&i2c_bus->cmd_complete);
+ break;
+ case AST2600_I2CM_NORMAL_STOP:
+ /* write 0 byte only have stop isr */
+ i2c_bus->msgs_index++;
+ if (i2c_bus->msgs_index < i2c_bus->msgs_count) {
+ if (ast2600_i2c_do_start(i2c_bus)) {
+ i2c_bus->cmd_err = -ENOMEM;
+ complete(&i2c_bus->cmd_complete);
+ }
+ } else {
+ i2c_bus->cmd_err = i2c_bus->msgs_index;
+ complete(&i2c_bus->cmd_complete);
+ }
+ break;
+ case AST2600_I2CM_TX_ACK:
+ case AST2600_I2CM_TX_ACK | AST2600_I2CM_NORMAL_STOP:
+ xfer_len = AST2600_I2CC_GET_TX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ i2c_bus->controller_xfer_cnt += xfer_len;
+
+ if (i2c_bus->controller_xfer_cnt == msg->len) {
+ i2c_bus->msgs_index++;
+ if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
+ i2c_bus->cmd_err = i2c_bus->msgs_index;
+ complete(&i2c_bus->cmd_complete);
+ } else {
+ if (ast2600_i2c_do_start(i2c_bus)) {
+ i2c_bus->cmd_err = -ENOMEM;
+ complete(&i2c_bus->cmd_complete);
+ }
+ }
+ } else {
+ ast2600_i2c_setup_buff_tx(0, i2c_bus);
+ }
+ break;
+ case AST2600_I2CM_RX_DONE:
+ case AST2600_I2CM_RX_DONE | AST2600_I2CM_NORMAL_STOP:
+ /* do next rx */
+ xfer_len = AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
+ AST2600_I2CC_BUFF_CTRL));
+ for (i = 0; i < xfer_len; i++)
+ msg->buf[i2c_bus->controller_xfer_cnt + i] =
+ readb(i2c_bus->buf_base + 0x10 + i);
+
+ if (msg->flags & I2C_M_RECV_LEN) {
+ u8 recv_len = AST2600_I2CC_GET_RX_BUFF(readl(i2c_bus->reg_base
+ + AST2600_I2CC_STS_AND_BUFF));
+ msg->len = min_t(unsigned int, recv_len, I2C_SMBUS_BLOCK_MAX);
+ msg->len += ((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
+ msg->flags &= ~I2C_M_RECV_LEN;
+ if (!recv_len)
+ i2c_bus->controller_xfer_cnt = 0;
+ else
+ i2c_bus->controller_xfer_cnt = 1;
+ } else {
+ i2c_bus->controller_xfer_cnt += xfer_len;
+ }
+
+ if (i2c_bus->controller_xfer_cnt == msg->len) {
+ i2c_bus->msgs_index++;
+ if (i2c_bus->msgs_index == i2c_bus->msgs_count) {
+ i2c_bus->cmd_err = i2c_bus->msgs_index;
+ complete(&i2c_bus->cmd_complete);
+ } else {
+ if (ast2600_i2c_do_start(i2c_bus)) {
+ i2c_bus->cmd_err = -ENOMEM;
+ complete(&i2c_bus->cmd_complete);
+ }
+ }
+ } else {
+ ast2600_i2c_setup_buff_rx(0, i2c_bus);
+ }
+ break;
+ default:
+ dev_dbg(i2c_bus->dev, "unhandled sts %x\n", sts);
+ break;
+ }
+}
+
+static int ast2600_i2c_controller_irq(struct ast2600_i2c_bus *i2c_bus)
+{
+ u32 sts = readl(i2c_bus->reg_base + AST2600_I2CM_ISR);
+ u32 ctrl;
+
+ sts &= ~AST2600_I2CM_SMBUS_ALERT;
+
+ if (sts & AST2600_I2CM_BUS_RECOVER_FAIL) {
+ writel(AST2600_I2CM_BUS_RECOVER_FAIL, i2c_bus->reg_base + AST2600_I2CM_ISR);
+ ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ i2c_bus->cmd_err = -EPROTO;
+ complete(&i2c_bus->cmd_complete);
+ return 1;
+ }
+
+ if (sts & AST2600_I2CM_BUS_RECOVER) {
+ writel(AST2600_I2CM_BUS_RECOVER, i2c_bus->reg_base + AST2600_I2CM_ISR);
+ i2c_bus->cmd_err = 0;
+ complete(&i2c_bus->cmd_complete);
+ return 1;
+ }
+
+ i2c_bus->cmd_err = ast2600_i2c_irq_err_to_errno(sts);
+ if (i2c_bus->cmd_err) {
+ writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base + AST2600_I2CM_ISR);
+ complete(&i2c_bus->cmd_complete);
+ return 1;
+ }
+
+ if (sts & AST2600_I2CM_PKT_DONE) {
+ ast2600_i2c_controller_packet_irq(i2c_bus, sts);
+ return 1;
+ }
+
+ return 0;
+}
+
+static irqreturn_t ast2600_i2c_bus_irq(int irq, void *dev_id)
+{
+ struct ast2600_i2c_bus *i2c_bus = dev_id;
+
+ return IRQ_RETVAL(ast2600_i2c_controller_irq(i2c_bus));
+}
+
+static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+ struct ast2600_i2c_bus *i2c_bus = i2c_get_adapdata(adap);
+ unsigned long timeout;
+ int ret;
+
+ if (!i2c_bus->multi_master &&
+ (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) & AST2600_I2CC_BUS_BUSY_STS)) {
+ ret = ast2600_i2c_recover_bus(i2c_bus);
+ if (ret)
+ return ret;
+ }
+
+ i2c_bus->cmd_err = 0;
+ i2c_bus->msgs = msgs;
+ i2c_bus->msgs_index = 0;
+ i2c_bus->msgs_count = num;
+ reinit_completion(&i2c_bus->cmd_complete);
+ ret = ast2600_i2c_do_start(i2c_bus);
+ if (ret)
+ goto controller_out;
+ timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete, i2c_bus->adap.timeout);
+ if (timeout == 0) {
+ u32 ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+ dev_dbg(i2c_bus->dev, "timeout isr[%x], sts[%x]\n",
+ readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
+ readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
+ writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+ /*
+ * A slave holding SCL low can stall the transfer and trigger
+ * a master timeout. In multi-master mode, attempt bus recovery
+ * if the bus is still busy.
+ */
+ if (i2c_bus->multi_master &&
+ (readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF) &
+ AST2600_I2CC_BUS_BUSY_STS))
+ ast2600_i2c_recover_bus(i2c_bus);
+ ret = -ETIMEDOUT;
+ } else {
+ ret = i2c_bus->cmd_err;
+ }
+
+ dev_dbg(i2c_bus->dev, "bus%d-m: %d end\n", i2c_bus->adap.nr, i2c_bus->cmd_err);
+
+controller_out:
+ return ret;
+}
+
+static int ast2600_i2c_init(struct ast2600_i2c_bus *i2c_bus)
+{
+ u32 fun_ctrl = AST2600_I2CC_BUS_AUTO_RELEASE | AST2600_I2CC_MASTER_EN;
+
+ /* I2C Reset */
+ writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+
+ if (!i2c_bus->multi_master)
+ fun_ctrl |= AST2600_I2CC_MULTI_MASTER_DIS;
+
+ /* Enable Controller Mode */
+ writel(fun_ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ /* disable target address */
+ writel(0, i2c_bus->reg_base + AST2600_I2CS_ADDR_CTRL);
+
+ /* Set AC Timing */
+ ast2600_i2c_ac_timing_config(i2c_bus);
+
+ /* Clear Interrupt */
+ writel(GENMASK(27, 0), i2c_bus->reg_base + AST2600_I2CM_ISR);
+
+ return 0;
+}
+
+static u32 ast2600_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm i2c_ast2600_algorithm = {
+ .xfer = ast2600_i2c_controller_xfer,
+ .functionality = ast2600_i2c_functionality,
+};
+
+static int ast2600_i2c_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct ast2600_i2c_bus *i2c_bus;
+ void __iomem *buf_base;
+ struct reset_control *rst;
+ struct resource *res;
+ u32 global_ctrl;
+ int ret;
+
+ if (!device_property_present(dev, "aspeed,global-regs"))
+ return -ENODEV;
+
+ i2c_bus = devm_kzalloc(dev, sizeof(*i2c_bus), GFP_KERNEL);
+ if (!i2c_bus)
+ return -ENOMEM;
+
+ i2c_bus->reg_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(i2c_bus->reg_base))
+ return PTR_ERR(i2c_bus->reg_base);
+
+ rst = devm_reset_control_get_shared_deasserted(dev, NULL);
+ if (IS_ERR(rst))
+ return dev_err_probe(dev, PTR_ERR(rst), "Missing reset ctrl\n");
+
+ i2c_bus->global_regs =
+ syscon_regmap_lookup_by_phandle(dev_of_node(dev), "aspeed,global-regs");
+ if (IS_ERR(i2c_bus->global_regs))
+ return PTR_ERR(i2c_bus->global_regs);
+
+ regmap_read(i2c_bus->global_regs, AST2600_I2CG_CTRL, &global_ctrl);
+ if ((global_ctrl & AST2600_GLOBAL_INIT) != AST2600_GLOBAL_INIT) {
+ regmap_write(i2c_bus->global_regs, AST2600_I2CG_CTRL, AST2600_GLOBAL_INIT);
+ regmap_write(i2c_bus->global_regs, AST2600_I2CG_CLK_DIV_CTRL, I2CCG_DIV_CTRL);
+ }
+
+ i2c_bus->dev = dev;
+ i2c_bus->multi_master = device_property_read_bool(dev, "multi-master");
+
+ buf_base = devm_platform_get_and_ioremap_resource(pdev, 1, &res);
+ if (IS_ERR(buf_base))
+ return dev_err_probe(dev, PTR_ERR(buf_base), "Missing buffer resource\n");
+ i2c_bus->buf_base = buf_base;
+ i2c_bus->buf_size = resource_size(res) / 2;
+
+ /*
+ * i2c timeout counter: use base clk4 1Mhz,
+ * per unit: 1/(1000/1024) = 1024us
+ */
+ ret = device_property_read_u32(dev, "i2c-scl-clk-low-timeout-us", &i2c_bus->timeout);
+ if (!ret)
+ i2c_bus->timeout = DIV_ROUND_UP(i2c_bus->timeout, 1024);
+
+ init_completion(&i2c_bus->cmd_complete);
+
+ i2c_bus->irq = platform_get_irq(pdev, 0);
+ if (i2c_bus->irq < 0)
+ return i2c_bus->irq;
+
+ platform_set_drvdata(pdev, i2c_bus);
+
+ i2c_bus->clk = devm_clk_get(i2c_bus->dev, NULL);
+ if (IS_ERR(i2c_bus->clk))
+ return dev_err_probe(i2c_bus->dev, PTR_ERR(i2c_bus->clk), "Can't get clock\n");
+
+ i2c_bus->apb_clk = clk_get_rate(i2c_bus->clk);
+
+ i2c_parse_fw_timings(i2c_bus->dev, &i2c_bus->timing_info, true);
+
+ /* Initialize the I2C adapter */
+ i2c_bus->adap.owner = THIS_MODULE;
+ i2c_bus->adap.algo = &i2c_ast2600_algorithm;
+ i2c_bus->adap.retries = 0;
+ i2c_bus->adap.dev.parent = i2c_bus->dev;
+ device_set_node(&i2c_bus->adap.dev, dev_fwnode(dev));
+ i2c_bus->adap.algo_data = i2c_bus;
+ strscpy(i2c_bus->adap.name, pdev->name);
+ i2c_set_adapdata(&i2c_bus->adap, i2c_bus);
+
+ ret = ast2600_i2c_init(i2c_bus);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Unable to initialize i2c %d\n", ret);
+
+ ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
+ dev_name(dev), i2c_bus);
+ if (ret < 0) {
+ ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
+ i2c_bus->irq);
+ goto err;
+ }
+
+ writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
+ i2c_bus->reg_base + AST2600_I2CM_IER);
+
+ ret = devm_i2c_add_adapter(dev, &i2c_bus->adap);
+ if (ret)
+ goto err;
+
+ return 0;
+
+err:
+ writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+ return ret;
+}
+
+static void ast2600_i2c_remove(struct platform_device *pdev)
+{
+ struct ast2600_i2c_bus *i2c_bus = platform_get_drvdata(pdev);
+
+ /* Disable everything. */
+ writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
+ writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
+}
+
+static const struct of_device_id ast2600_i2c_of_match[] = {
+ { .compatible = "aspeed,ast2600-i2c-bus" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, ast2600_i2c_of_match);
+
+static struct platform_driver ast2600_i2c_driver = {
+ .probe = ast2600_i2c_probe,
+ .remove = ast2600_i2c_remove,
+ .driver = {
+ .name = "ast2600-i2c",
+ .of_match_table = ast2600_i2c_of_match,
+ },
+};
+module_platform_driver(ast2600_i2c_driver);
+
+MODULE_AUTHOR("Ryan Chen <ryan_chen@aspeedtech.com>");
+MODULE_DESCRIPTION("ASPEED AST2600 I2C Controller Driver");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related
* [PATCH RESEND v29 2/4] dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs properties
From: Ryan Chen @ 2026-05-13 5:32 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen
In-Reply-To: <20260513-upstream_i2c-v29-0-fe9926964d55@aspeedtech.com>
Add the aspeed,global-regs phandle to reference the AST2600 global
registers syscon node, containing the SoC-common I2C register set.
These properties apply only to the AST2600 binding. Legacy DTs remain
unchanged.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v29:
- remove aspeed,enable-dma properties.
Changes in v28:
- update commit message correspond with aspeed,enable-dma.
- remove aspeed,transfer-mode and add aspeed,enable-dma property and
description.
- Fix aspeed,enable-dma description to reflect hardware capability rather
than software behavior
Changes in v27:
- change aspeed,transfer-mode to aspeed,enable-dma.
---
Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
index de2c359037da..0c769efb76a5 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -37,6 +37,12 @@ properties:
resets:
maxItems: 1
+ aspeed,global-regs:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle reference to the i2c global syscon node, containing the
+ SoC-common i2c register set.
+
required:
- reg
- compatible
@@ -59,4 +65,5 @@ examples:
resets = <&syscon ASPEED_RESET_I2C>;
clock-frequency = <100000>;
interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+ aspeed,global-regs = <&i2c_global>;
};
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox