* Re: [PATCH 07/12] clk: qcom: gcc-mdm9607: Drop incorrect apss_tcu_clk_src
From: Stephan Gerhold @ 2026-06-10 14:09 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Georgi Djakov,
Shawn Guo, Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <f55526ad-ecd2-47b2-8b8f-5aec429bc1c0@oss.qualcomm.com>
On Wed, Jun 10, 2026 at 03:50:11PM +0200, Konrad Dybcio wrote:
> On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> > From: Stephan Gerhold <stephan@gerhold.net>
> >
> > This clock does not seem to exist on MDM9607. Reading/writing the registers
> > always results in 0 or crashes. The math in the frequency table is also
> > broken. GPLL2 on MDM9607 runs at 480 MHz, so:
> >
> > - F(155000000, P_GPLL2, 6, 0, 0), // 480 MHz/6 = 80 MHz, not 155 MHz
> > - F(310000000, P_GPLL2, 3, 0, 0), // 480 MHz/3 = 160 MHz, not 310 MHz
> >
> > Presumably, this definition was mistakenly copied as-is from gcc-msm8916
> > (which uses 930 MHz for GPLL2). There are no branch consumers of this root
> > clock inside gcc-mdm9607 (notably, gcc_apss_tcu_clk has bimc_ddr_clk_src as
> > parent instead of this clock), so we can just drop it.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
>
> It does, CMD_RCGR @ 0x37000
>
> Interestingly, there's also an SMMU instance.. although downstream doesn't
> seem bothered with it
>
Thanks for checking, but that still doesn't give us enough to work with
it. In particular:
- What are the frequency tables? Which parents exist with which IDs?
- What is the hid_width?
- Where do we hook it up to some children?
The way it is right now it's definitely wrong. I don't think we'll need
it so removing it like this patch is probably the easiest thing to do...
Thanks,
Stephan
PS: This platform has the weird TZ-managed SMMU (like MSM8916), so there
isn't much you can do with it from Linux. Unless you are running TF-A,
then you can describe it as "arm,mmu-500" and use it for everything
pretty much exactly like you would normally expect. But you still don't
need to do anything with this clock.
^ permalink raw reply
* Re: [PATCH v2 02/16] device property: Add fwnode_graph_get_next_port_endpoint()
From: Andy Shevchenko @ 2026-06-10 14:08 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Alan Stern,
linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam
In-Reply-To: <20260610084053.2059858-3-wenst@chromium.org>
On Wed, Jun 10, 2026 at 04:40:36PM +0800, Chen-Yu Tsai wrote:
> Due to design constraints of the power sequencing API, the consumer
> must first be sure that the other side is actually a provider, or it
> will continually get -EPROBE_DEFER when requesting the power
> sequencing descriptor.
>
> In the upcoming USB power sequencing integration, the USB hub driver
> first needs to check whether a graph connection exists, and whether
> the other side of the connection is a supported connector type. The
> USB port is tied to a "port" firmware node, and this new helper will
> be used to get the endpoint under the known "port" firmware node.
...
> +/**
> + * fwnode_graph_get_next_port_endpoint - Get next endpoint firmware node in port
> + * @port: Pointer to the target port firmware node
> + * @prev: Previous endpoint node or %NULL to get the first
> + *
> + * The caller is responsible for calling fwnode_handle_put() on the returned
> + * fwnode pointer. Note that this function also puts a reference to @prev
> + * unconditionally.
> + *
> + * Return: an endpoint firmware node pointer or %NULL if no more endpoints
> + * are available.
Yeah, you see, even here is inconsistency with previously added kernel-doc.
> + */
> +struct fwnode_handle *fwnode_graph_get_next_port_endpoint(const struct fwnode_handle *port,
> + struct fwnode_handle *prev)
> +{
> + struct fwnode_handle *ep;
Unused?
> + while (1) {
This is usually harder to read and follow. It's like "pay much attention on
the code", but here no rocket science, no code to really pay attention to.
> + prev = fwnode_get_next_child_node(port, prev);
> + if (!prev)
> + break;
> +
> + if (WARN(!fwnode_name_eq(prev, "endpoint"),
> + "non endpoint node is used (%pfw)", prev))
> + continue;
> +
> + break;
> + }
> +
> + return prev;
> +}
So, this can be rewritten as
ep = prev;
do {
ep = fwnode_get_next_child_node(port, ep);
if (fwnode_name_eq(ep, "endpoint"))
break;
WARN_ON(ep, ...);
} while (ep);
return ep;
But also big question why? to WARN*(). There is no use in the entire
property.c.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v4 phy-next 14/16] dt-bindings: phy: lynx-10g: initial document
From: Vladimir Oltean @ 2026-06-10 14:05 UTC (permalink / raw)
To: Conor Dooley
Cc: linux-phy, Ioana Ciornei, Vinod Koul, Neil Armstrong,
Tanjeff Moos, linux-kernel, Rob Herring, devicetree, Conor Dooley,
Krzysztof Kozlowski
In-Reply-To: <20260609-panda-breeding-c34f8c9224bd@spud>
On Tue, Jun 09, 2026 at 05:26:17PM +0100, Conor Dooley wrote:
> On Tue, Jun 09, 2026 at 01:17:08AM +0300, Vladimir Oltean wrote:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,ls1046a-serdes1
> > + - fsl,ls1046a-serdes2
> > + then:
>
> > + properties:
> > + big-endian: true
>
> This should have stayed at the top-level FWIW, and here you would do
> then:
> required:
> - big-endian
> else:
> properties:
> big-endian: false
Thanks, I'll make both changes for v5.
^ permalink raw reply
* Re: [PATCH v4 12/13] arm64: dts: qcom: agatti: Add DSI1 PHY and sleep clocks to DISPCC node
From: Konrad Dybcio @ 2026-06-10 14:02 UTC (permalink / raw)
To: Imran Shaik, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Loic Poulain, Brian Masney
Cc: Ajit Pandey, Taniya Das, Jagadeesh Kona, linux-arm-msm, linux-clk,
devicetree, linux-kernel
In-Reply-To: <20260604-shikra-dispcc-gpucc-v4-12-8204f1029311@oss.qualcomm.com>
On 6/4/26 7:26 AM, Imran Shaik wrote:
> Update the DISPCC node on QCM2290 (Agatti) to align with the latest DT
> bindings changes, which adds support for the DSI1 PHY and sleep clocks.
>
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
The Agatti MDSS doc suggests there's only a single DSI controller
and PHY. There are registers to support a secondary PHY, but
are those just leftovers?
Konrad
^ permalink raw reply
* Re: [PATCH v4 11/13] clk: qcom: Add support for Qualcomm GPU Clock Controller on Shikra
From: Konrad Dybcio @ 2026-06-10 14:00 UTC (permalink / raw)
To: Imran Shaik, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Loic Poulain, Brian Masney
Cc: Ajit Pandey, Taniya Das, Jagadeesh Kona, linux-arm-msm, linux-clk,
devicetree, linux-kernel
In-Reply-To: <20260604-shikra-dispcc-gpucc-v4-11-8204f1029311@oss.qualcomm.com>
On 6/4/26 7:26 AM, Imran Shaik wrote:
> The Qualcomm Shikra GPU clock controller is similar to QCM2290 GPUCC
> hardware block, with minor differences. Hence add support for Shikra
> GPUCC by extending the QCM2290 GPUCC driver.
>
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v4 09/13] clk: qcom: gpucc-qcm2290: Park RCG's clk source at XO during disable
From: Konrad Dybcio @ 2026-06-10 14:00 UTC (permalink / raw)
To: Imran Shaik, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Loic Poulain, Brian Masney
Cc: Ajit Pandey, Taniya Das, Jagadeesh Kona, linux-arm-msm, linux-clk,
devicetree, linux-kernel
In-Reply-To: <20260604-shikra-dispcc-gpucc-v4-9-8204f1029311@oss.qualcomm.com>
On 6/4/26 7:26 AM, Imran Shaik wrote:
> The RCG's clk src has to be parked at XO while disabling as per the
> HW recommendation, hence use clk_rcg2_shared_ops to achieve the same.
>
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
This RCG isn't marked as "safe" downstream, was it overlooked there
too?
In any case, this needs a Fixes tag
Konrad
^ permalink raw reply
* Re: [PATCH v10 5/6] pinctrl: s32cc: implement GPIO functionality
From: Khristine Andreea Barbulescu @ 2026-06-10 13:58 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Chester Lin, Matthias Brugger,
Ghennadi Procopciuc, Larisa Grigore, Lee Jones, Shawn Guo,
Sascha Hauer, Fabio Estevam, Dong Aisheng, Jacky Bai,
Greg Kroah-Hartman, Rafael J. Wysocki, Srinivas Kandagatla,
Alberto Ruiz, Christophe Lizzi, devicetree, Eric Chanudet, imx,
linux-arm-kernel, linux-gpio, linux-kernel, NXP S32 Linux Team,
Pengutronix Kernel Team, Vincent Guittot
In-Reply-To: <CALE0LRu1aJAY_-7imYFFPbEwWPpodArXbxtjE-ur3UQnVt5fHw@mail.gmail.com>
On 6/2/2026 12:26 PM, Enric Balletbo i Serra wrote:
> Hi Khristine,
>
> Thank you for the patch. I got some checkpatch warnings, could you
> take a look? And some minor comments below.
>
> On Tue, Jun 2, 2026 at 10:02 AM Khristine Andreea Barbulescu
> <khristineandreea.barbulescu@oss.nxp.com> wrote:
>>
>> From: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>>
>> The updated SIUL2 block groups pinctrl, GPIO data access
>> and interrupt control within the same hardware unit.
>> The SIUL2 driver is therefore structured as a monolithic
>> pinctrl/GPIO driver.
>>
>> GPIO data access and direction handling are implemented using the
>> gpio-regmap library backed by a virtual regmap. The virtual regmap
>> translates the gpio-regmap register model to the underlying SIUL2
>> registers: MSCR for direction, PGPDI for input values and PGPDO for
>> output values.
>>
>> The existing pinctrl GPIO callbacks are used for the request/free path:
>> they switch the pad to GPIO mode on request and restore the previous
>> MSCR configuration when the GPIO is released.
>>
>> This change came as a result of upstream review in the
>> following series:
>> https://lore.kernel.org/linux-gpio/20260120115923.3463866-4-khristineandreea.barbulescu@oss.nxp.com/T/#m543c9edbdde74bdc68b6a2364e8b975356c33043
>> https://lore.kernel.org/all/20260504131148.3622697-7-khristineandreea.barbulescu@oss.nxp.com/
>>
>> Support both SIUL2 DT layouts:
>> - legacy pinctrl-only binding
>> - extended pinctrl/GPIO/irqchip binding
>>
>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
>> Signed-off-by: Khristine Andreea Barbulescu <khristineandreea.barbulescu@oss.nxp.com>
>> ---
>> drivers/pinctrl/nxp/Kconfig | 1 +
>> drivers/pinctrl/nxp/pinctrl-s32.h | 32 +-
>> drivers/pinctrl/nxp/pinctrl-s32cc.c | 685 +++++++++++++++++++++++++---
>> drivers/pinctrl/nxp/pinctrl-s32g2.c | 46 +-
>> 4 files changed, 686 insertions(+), 78 deletions(-)
>>
>> diff --git a/drivers/pinctrl/nxp/Kconfig b/drivers/pinctrl/nxp/Kconfig
>> index abca7ef97003..59fc6adf5b0b 100644
>> --- a/drivers/pinctrl/nxp/Kconfig
>> +++ b/drivers/pinctrl/nxp/Kconfig
>> @@ -5,6 +5,7 @@ config PINCTRL_S32CC
>> select GENERIC_PINCTRL_GROUPS
>> select GENERIC_PINMUX_FUNCTIONS
>> select GENERIC_PINCONF
>> + select GPIO_REGMAP
>> select REGMAP_MMIO
>>
>> config PINCTRL_S32G2
>> diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
>> index 8715befd5f05..c2fc5eda7eb4 100644
>> --- a/drivers/pinctrl/nxp/pinctrl-s32.h
>> +++ b/drivers/pinctrl/nxp/pinctrl-s32.h
>> @@ -2,7 +2,7 @@
>> *
>> * S32 pinmux core definitions
>> *
>> - * Copyright 2016-2020, 2022 NXP
>> + * Copyright 2016-2020, 2022, 2026 NXP
>> * Copyright (C) 2022 SUSE LLC
>> * Copyright 2015-2016 Freescale Semiconductor, Inc.
>> * Copyright (C) 2012 Linaro Ltd.
>> @@ -34,11 +34,39 @@ struct s32_pin_range {
>> unsigned int end;
>> };
>>
>> +/**
>> + * struct s32_gpio_range - contiguous GPIO pin range within a SIUL2 module
>> + * @gpio_base: first GPIO line offset in the GPIO range
>> + * @pin_base: first pinctrl pin number mapped by this GPIO range
>> + * @gpio_num: number of consecutive GPIO pins in the range
>> + */
>> +struct s32_gpio_range {
>> + unsigned int gpio_base;
>> + unsigned int pin_base;
>> + unsigned int gpio_num;
>> +};
>> +
>> +/**
>> + * struct s32_gpio_pad_map - mapping between GPIO ranges and PGPD pads
>> + * @gpio_start: first GPIO line offset in the range
>> + * @gpio_end: last GPIO line offset in the range
>> + * @pad: PGPD pad number serving the range
>> + */
>> +struct s32_gpio_pad_map {
>> + unsigned int gpio_start;
>> + unsigned int gpio_end;
>> + unsigned int pad;
>> +};
>> +
>> struct s32_pinctrl_soc_data {
>> const struct pinctrl_pin_desc *pins;
>> unsigned int npins;
>> const struct s32_pin_range *mem_pin_ranges;
>> unsigned int mem_regions;
>> + const struct s32_gpio_range *gpio_ranges;
>> + unsigned int num_gpio_ranges;
>> + const struct s32_gpio_pad_map *gpio_pad_maps;
>> + unsigned int num_gpio_pad_maps;
>> };
>>
>> struct s32_pinctrl_soc_info {
>> @@ -53,6 +81,8 @@ struct s32_pinctrl_soc_info {
>>
>> #define S32_PINCTRL_PIN(pin) PINCTRL_PIN(pin, #pin)
>> #define S32_PIN_RANGE(_start, _end) { .start = _start, .end = _end }
>> +#define S32_GPIO_RANGE(gpio, pin, num) \
>> + { .gpio_base = gpio, .pin_base = pin, .gpio_num = num }
>>
>> int s32_pinctrl_probe(struct platform_device *pdev,
>> const struct s32_pinctrl_soc_data *soc_data);
>> diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
>> index 89a4eb2000ee..8843926345ec 100644
>> --- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
>> +++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
>> @@ -2,7 +2,7 @@
>> /*
>> * Core driver for the S32 CC (Common Chassis) pin controller
>> *
>> - * Copyright 2017-2022,2024 NXP
>> + * Copyright 2017-2022,2024-2026 NXP
>> * Copyright (C) 2022 SUSE LLC
>> * Copyright 2015-2016 Freescale Semiconductor, Inc.
>> */
>> @@ -10,6 +10,7 @@
>> #include <linux/bitops.h>
>> #include <linux/err.h>
>> #include <linux/gpio/driver.h>
>> +#include <linux/gpio/regmap.h>
>> #include <linux/init.h>
>> #include <linux/io.h>
>> #include <linux/module.h>
>> @@ -39,6 +40,40 @@
>> #define S32_MSCR_ODE BIT(20)
>> #define S32_MSCR_OBE BIT(21)
>>
>> +#define S32_GPIO_OP_SHIFT 16
>> +#define S32_GPIO_OP_MASK GENMASK(19, 16)
>> +
>> +#define S32_GPIO_OP_DIR 0 /* MSCR direction */
>> +#define S32_GPIO_OP_DAT BIT(S32_GPIO_OP_SHIFT) /* PGPDI read */
>> +#define S32_GPIO_OP_SET BIT(S32_GPIO_OP_SHIFT + 1) /* PGPDO write */
>> +
>> +/*
>> + * [15:12] = GPIO bank / gpio range index
>> + * [11:0] = real register offset or pin id
>> + */
>> +#define S32_GPIO_BANK_SHIFT 12
>> +#define S32_GPIO_BANK_MASK GENMASK(15, 12)
>> +#define S32_GPIO_REG_MASK GENMASK(11, 0)
>> +
>> +#define S32_GPIO_ENCODE(bank, off) \
>> + ((((bank) << S32_GPIO_BANK_SHIFT) & S32_GPIO_BANK_MASK) | \
>> + ((off) & S32_GPIO_REG_MASK))
>> +
>> +#define S32_GPIO_DECODE_BANK(reg) \
>> + (((reg) & S32_GPIO_BANK_MASK) >> S32_GPIO_BANK_SHIFT)
>> +
>> +#define S32_GPIO_DECODE_OFF(reg) \
>> + ((reg) & S32_GPIO_REG_MASK)
>> +
>> +/*
>> + * PGPDOs are 16bit registers that come in big endian
>> + * order if they are grouped in pairs of two.
>> + *
>> + * For example, the order is PGPDO1, PGPDO0, PGPDO3, PGPDO2...
>> + */
>> +#define S32_PGPD(N) (((N) ^ 1) * 2)
>> +#define S32_PGPD_SIZE 16
>> +
>> enum s32_write_type {
>> S32_PINCONF_UPDATE_ONLY,
>> S32_PINCONF_OVERWRITE,
>> @@ -72,6 +107,18 @@ struct s32_pinctrl_mem_region {
>> char name[8];
>> };
>>
>> +/*
>> + * struct s32_gpio_regmaps - GPIO register maps for a SIUL2 instance
>> + * @pgpdo: regmap for Parallel GPIO Pad Data Out registers
>> + * @pgpdi: regmap for Parallel GPIO Pad Data In registers
>> + * @range: GPIO range info
>> + */
>> +struct s32_gpio_regmaps {
>> + struct regmap *pgpdo;
>> + struct regmap *pgpdi;
>> + const struct s32_gpio_range *range;
>> +};
>> +
>> /*
>> * struct gpio_pin_config - holds pin configuration for GPIO's
>> * @pin_id: Pin ID for this GPIO
>> @@ -98,6 +145,12 @@ struct s32_pinctrl_context {
>> * @pctl: a pointer to the pinctrl device structure
>> * @regions: reserved memory regions with start/end pin
>> * @info: structure containing information about the pin
>> + * @gpio_regmaps: PGPDO/PGPDI regmaps for each SIUL2 module
>> + * @num_gpio_regmaps: number of GPIO regmap entries
>> + * @gpio_regmap: regmap bridging gpio-regmap to SIUL2 registers
>> + * @gpio_rgm: gpio-regmap instance registered for this controller
>> + * @ngpio: total number of GPIO line offsets
>> + * @gpio_names: GPIO line names array passed to gpio-regmap
>> * @gpio_configs: saved configurations for GPIO pins
>> * @gpio_configs_lock: lock for the `gpio_configs` list
>> * @saved_context: configuration saved over system sleep
>> @@ -107,6 +160,12 @@ struct s32_pinctrl {
>> struct pinctrl_dev *pctl;
>> struct s32_pinctrl_mem_region *regions;
>> struct s32_pinctrl_soc_info *info;
>> + struct s32_gpio_regmaps *gpio_regmaps;
>> + unsigned int num_gpio_regmaps;
>> + struct regmap *gpio_regmap;
>> + struct gpio_regmap *gpio_rgm;
>> + unsigned int ngpio;
>> + const char *const *gpio_names;
>> struct list_head gpio_configs;
>> spinlock_t gpio_configs_lock;
>> #ifdef CONFIG_PM_SLEEP
>> @@ -356,88 +415,84 @@ static int s32_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
>> return info->nfunctions;
>> }
>>
>> -static const char *s32_pmx_get_func_name(struct pinctrl_dev *pctldev,
>> - unsigned int selector)
>> -{
>> - struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> - const struct s32_pinctrl_soc_info *info = ipctl->info;
>> -
>> - return info->functions[selector].name;
>> -}
>> -
>> -static int s32_pmx_get_groups(struct pinctrl_dev *pctldev,
>> - unsigned int selector,
>> - const char * const **groups,
>> - unsigned int * const num_groups)
>> -{
>> - struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> - const struct s32_pinctrl_soc_info *info = ipctl->info;
>> -
>> - *groups = info->functions[selector].groups;
>> - *num_groups = info->functions[selector].ngroups;
>> -
>> - return 0;
>> -}
>> -
>> static int s32_pmx_gpio_request_enable(struct pinctrl_dev *pctldev,
>> struct pinctrl_gpio_range *range,
>> - unsigned int offset)
>> + unsigned int pin)
>> {
>> struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> - struct gpio_pin_config *gpio_pin;
>> + struct gpio_pin_config *gpio_pin __free(kfree) = NULL;
>> unsigned int config;
>> - unsigned long flags;
>> int ret;
>>
>> - ret = s32_regmap_read(pctldev, offset, &config);
>> + ret = s32_regmap_read(pctldev, pin, &config);
>> if (ret)
>> return ret;
>>
>> - /* Save current configuration */
>> - gpio_pin = kmalloc_obj(*gpio_pin);
>> + gpio_pin = kmalloc(sizeof(*gpio_pin), GFP_KERNEL);
>
> Why? Isn't kmalloc_obj safer?
>
>> if (!gpio_pin)
>> return -ENOMEM;
>>
>> - gpio_pin->pin_id = offset;
>> + gpio_pin->pin_id = pin;
>> gpio_pin->config = config;
>> - INIT_LIST_HEAD(&gpio_pin->list);
>> -
>> - spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
>> - list_add(&gpio_pin->list, &ipctl->gpio_configs);
>> - spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
>>
>> /* GPIO pin means SSS = 0 */
>> - config &= ~S32_MSCR_SSS_MASK;
>> + ret = s32_regmap_update(pctldev, pin,
>> + S32_MSCR_SSS_MASK | S32_MSCR_IBE,
>> + S32_MSCR_IBE);
>> + if (ret)
>> + return ret;
>> +
>> + scoped_guard(spinlock_irqsave, &ipctl->gpio_configs_lock)
>> + list_add(&no_free_ptr(gpio_pin)->list, &ipctl->gpio_configs);
>>
>> - return s32_regmap_write(pctldev, offset, config);
>> + return 0;
>> }
>>
>> static void s32_pmx_gpio_disable_free(struct pinctrl_dev *pctldev,
>> struct pinctrl_gpio_range *range,
>> - unsigned int offset)
>> + unsigned int pin)
>> {
>> struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> - struct gpio_pin_config *gpio_pin, *tmp;
>> + struct gpio_pin_config *gpio_pin, *found = NULL;
>> unsigned long flags;
>> - int ret;
>>
>> spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
>> -
>> - list_for_each_entry_safe(gpio_pin, tmp, &ipctl->gpio_configs, list) {
>> - if (gpio_pin->pin_id == offset) {
>> - ret = s32_regmap_write(pctldev, gpio_pin->pin_id,
>> - gpio_pin->config);
>> - if (ret != 0)
>> - goto unlock;
>> -
>> + list_for_each_entry(gpio_pin, &ipctl->gpio_configs, list) {
>> + if (gpio_pin->pin_id == pin) {
>> list_del(&gpio_pin->list);
>> - kfree(gpio_pin);
>> + found = gpio_pin;
>> break;
>> }
>> }
>> -
>> -unlock:
>> spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
>> +
>> + if (found) {
>> + s32_regmap_write(pctldev, found->pin_id, found->config);
>> + kfree(found);
>> + }
>> +}
>> +
>> +static const char *s32_pmx_get_func_name(struct pinctrl_dev *pctldev,
>> + unsigned int selector)
>> +{
>> + struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> + const struct s32_pinctrl_soc_info *info = ipctl->info;
>> +
>> + return info->functions[selector].name;
>> +}
>> +
>> +static int s32_pmx_get_groups(struct pinctrl_dev *pctldev,
>> + unsigned int selector,
>> + const char * const **groups,
>> + unsigned int * const num_groups)
>> +{
>> + struct s32_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
>> + const struct s32_pinctrl_soc_info *info = ipctl->info;
>> +
>> + *groups = info->functions[selector].groups;
>> + *num_groups = info->functions[selector].ngroups;
>> +
>> + return 0;
>> }
>>
>> static int s32_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
>> @@ -649,9 +704,9 @@ static void s32_pinconf_dbg_show(struct pinctrl_dev *pctldev,
>>
>> ret = s32_regmap_read(pctldev, pin_id, &config);
>> if (ret)
>> - return;
>> -
>> - seq_printf(s, "0x%x", config);
>> + seq_printf(s, "error %d", ret);
>> + else
>> + seq_printf(s, "0x%x", config);
>> }
>>
>> static void s32_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>> @@ -662,15 +717,13 @@ static void s32_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
>> struct s32_pin_group *grp;
>> unsigned int config;
>> const char *name;
>> - int i, ret;
>> + int i;
>>
>> seq_puts(s, "\n");
>> grp = &info->groups[selector];
>> for (i = 0; i < grp->data.npins; i++) {
>> name = pin_get_name(pctldev, grp->data.pins[i]);
>> - ret = s32_regmap_read(pctldev, grp->data.pins[i], &config);
>> - if (ret)
>> - return;
>> + s32_regmap_read(pctldev, grp->data.pins[i], &config);
>> seq_printf(s, "%s: 0x%x\n", name, config);
>> }
>> }
>> @@ -683,6 +736,450 @@ static const struct pinconf_ops s32_pinconf_ops = {
>> .pin_config_group_dbg_show = s32_pinconf_group_dbg_show,
>> };
>>
>> +static void s32_gpio_free_saved_configs(void *data)
>> +{
>> + struct s32_pinctrl *ipctl = data;
>> + struct gpio_pin_config *gpio_pin, *tmp;
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&ipctl->gpio_configs_lock, flags);
>> + list_for_each_entry_safe(gpio_pin, tmp, &ipctl->gpio_configs, list) {
>> + list_del(&gpio_pin->list);
>> + kfree(gpio_pin);
>> + }
>> + spin_unlock_irqrestore(&ipctl->gpio_configs_lock, flags);
>> +}
>> +
>> +static unsigned int s32_pin2pad(unsigned int pin)
>> +{
>> + return pin / S32_PGPD_SIZE;
>> +}
>> +
>> +static u16 s32_pin2mask(unsigned int pin)
>> +{
>> + /*
>> + * From Reference manual :
>> + * PGPDOx[PPDOy] = GPDO(x × 16) + (15 - y)[PDO_(x × 16) + (15 - y)]
>> + */
>> + return BIT(S32_PGPD_SIZE - 1 - pin % S32_PGPD_SIZE);
>> +}
>> +
>> +static int s32_gpio_get_range(struct s32_pinctrl *ipctl,
>> + unsigned int gpio,
>> + unsigned int *pin,
>> + unsigned int *bank)
>> +{
>> + const struct s32_pinctrl_soc_data *soc_data = ipctl->info->soc_data;
>> + const struct s32_gpio_range *range;
>> + int i;
>> +
>> + for (i = 0; i < soc_data->num_gpio_ranges; i++) {
>> + range = &soc_data->gpio_ranges[i];
>> +
>> + if (gpio < range->gpio_base ||
>> + gpio >= range->gpio_base + range->gpio_num)
>> + continue;
>> +
>> + if (pin)
>> + *pin = range->pin_base + gpio - range->gpio_base;
>> +
>> + if (bank)
>> + *bank = i;
>> +
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int s32_gpio_pad_map_xlate(struct s32_pinctrl *ipctl,
>> + unsigned int gpio,
>> + unsigned int *reg_offset,
>> + u16 *mask)
>> +{
>> + const struct s32_pinctrl_soc_data *soc_data = ipctl->info->soc_data;
>> + const struct s32_gpio_pad_map *map;
>> + unsigned int bit;
>> + int i;
>> +
>> + if (!soc_data->gpio_pad_maps || !soc_data->num_gpio_pad_maps)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < soc_data->num_gpio_pad_maps; i++) {
>> + map = &soc_data->gpio_pad_maps[i];
>> +
>> + if (gpio < map->gpio_start || gpio > map->gpio_end)
>> + continue;
>> +
>> + bit = gpio - map->gpio_start;
>> + *mask = BIT(S32_PGPD_SIZE - 1 - bit);
>> + *reg_offset = S32_PGPD(map->pad);
>> +
>> + return 0;
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int s32_gpio_xlate_pgpd(struct s32_pinctrl *ipctl,
>> + unsigned int pin,
>> + unsigned int *reg_offset,
>> + u16 *mask)
>> +{
>> + /*
>> + * SIUL2_1 does not expose GPIO data registers as a linear pad sequence.
>> + * Valid PGPD offsets there correspond to PGPD7, PGPD9, PGPD10, PGPD11.
>> + */
>> + if (pin >= 112)
>
> The magic number 112 requires better documentation or a define
>
> Or maybe, (NOT TESTED) instead of hardcoding, check if a pad map
> exists for this pin
>
> /* Try pad map first (needed for SIUL2_1's sparse layout) */
> ret = s32_gpio_pad_map_xlate(ipctl, pin, reg_offset, mask);
> if (ret != -EINVAL)
> return ret;
>
> /* Fall back to linear layout (SIUL2_0) */
> *mask = s32_pin2mask(pin);
> *reg_offset = S32_PGPD(s32_pin2pad(pin));
> return 0;
>
> Does it make sense?
>
>> + return s32_gpio_pad_map_xlate(ipctl, pin, reg_offset, mask);
>> +
>> + *mask = s32_pin2mask(pin);
>> + *reg_offset = S32_PGPD(s32_pin2pad(pin));
>> +
>> + return 0;
>> +}
>> +
>> +static int s32_gpio_reg_mask_xlate(struct gpio_regmap *gpio,
>> + unsigned int base, unsigned int offset,
>> + unsigned int *reg, unsigned int *mask)
>> +{
>> + struct s32_pinctrl *ipctl = gpio_regmap_get_drvdata(gpio);
>> + unsigned int pgpd_reg, pin, bank;
>> + u16 pgpd_mask;
>> + int ret;
>> +
>> + ret = s32_gpio_get_range(ipctl, offset, &pin, &bank);
>> + if (ret)
>> + return ret;
>> +
>> + switch (base) {
>> + case S32_GPIO_OP_DIR:
>> + /*
>> + * Direction is controlled through MSCR OBE.
>> + * Encode the real pin id in the virtual register.
>> + */
>> + *reg = S32_GPIO_OP_DIR | pin;
>> + *mask = S32_MSCR_OBE;
>> + return 0;
>> +
>> + case S32_GPIO_OP_DAT:
>> + case S32_GPIO_OP_SET:
>> + ret = s32_gpio_xlate_pgpd(ipctl, pin, &pgpd_reg, &pgpd_mask);
>> + if (ret)
>> + return ret;
>> + /*
>> + * Encode both the GPIO bank and the real PGPD register offset.
>> + */
>> + *reg = base | S32_GPIO_ENCODE(bank, pgpd_reg);
>> + *mask = pgpd_mask;
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int s32_gpio_reg_read(void *context, unsigned int reg,
>> + unsigned int *val)
>> +{
>> + struct s32_pinctrl *ipctl = context;
>> + unsigned int op = reg & S32_GPIO_OP_MASK;
>> + unsigned int vreg = reg & ~S32_GPIO_OP_MASK;
>> + unsigned int bank;
>> + unsigned int offset;
>> + struct regmap *map;
>> +
>> + switch (op) {
>> + case S32_GPIO_OP_DIR:
>> + /*
>> + * Lower bits contain the real MSCR pin id.
>> + */
>> + offset = S32_GPIO_DECODE_OFF(vreg);
>> +
>> + return s32_regmap_read(ipctl->pctl, offset, val);
>> +
>> + case S32_GPIO_OP_DAT:
>> + bank = S32_GPIO_DECODE_BANK(vreg);
>> + offset = S32_GPIO_DECODE_OFF(vreg);
>> +
>> + if (bank >= ipctl->num_gpio_regmaps)
>> + return -EINVAL;
>> +
>> + map = ipctl->gpio_regmaps[bank].pgpdi;
>> + if (!map)
>> + return -ENODEV;
>> +
>> + return regmap_read(map, offset, val);
>> +
>> + case S32_GPIO_OP_SET:
>> + /*
>> + * gpio-regmap uses update_bits() for set, so it needs to read
>> + * the output register before writing the updated value.
>> + */
>> + bank = S32_GPIO_DECODE_BANK(vreg);
>> + offset = S32_GPIO_DECODE_OFF(vreg);
>> +
>> + if (bank >= ipctl->num_gpio_regmaps)
>> + return -EINVAL;
>> +
>> + map = ipctl->gpio_regmaps[bank].pgpdo;
>> + if (!map)
>> + return -ENODEV;
>> +
>> + return regmap_read(map, offset, val);
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int s32_gpio_reg_write(void *context, unsigned int reg,
>> + unsigned int val)
>> +{
>> + struct s32_pinctrl *ipctl = context;
>> + unsigned int op = reg & S32_GPIO_OP_MASK;
>> + unsigned int vreg = reg & ~S32_GPIO_OP_MASK;
>> + unsigned int bank, offset, config;
>> + struct regmap *map;
>> +
>> + switch (op) {
>> + case S32_GPIO_OP_DIR:
>> + /*
>> + * gpio-regmap sets S32_MSCR_OBE for output and clears it for
>> + * input. Keep IBE enabled for GPIOs in both cases.
>> + */
>> + offset = S32_GPIO_DECODE_OFF(vreg);
>> +
>> + config = S32_MSCR_IBE;
>> + if (val & S32_MSCR_OBE)
>> + config |= S32_MSCR_OBE;
>> +
>> + return s32_regmap_update(ipctl->pctl, offset,
>> + S32_MSCR_OBE | S32_MSCR_IBE,
>> + config);
>> +
>> + case S32_GPIO_OP_SET:
>> + bank = S32_GPIO_DECODE_BANK(vreg);
>> + offset = S32_GPIO_DECODE_OFF(vreg);
>> +
>> + if (bank >= ipctl->num_gpio_regmaps)
>> + return -EINVAL;
>> +
>> + map = ipctl->gpio_regmaps[bank].pgpdo;
>> + if (!map)
>> + return -ENODEV;
>> +
>> + return regmap_write(map, offset, val);
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static const struct regmap_bus s32_gpio_regmap_bus = {
>> + .reg_read = s32_gpio_reg_read,
>> + .reg_write = s32_gpio_reg_write,
>> +};
>> +
>> +static const struct regmap_config s32_gpio_regmap_config = {
>> + .name = "s32-gpio",
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 1,
>> + .max_register = S32_GPIO_OP_SET | S32_GPIO_BANK_MASK | S32_GPIO_REG_MASK,
>> + .cache_type = REGCACHE_NONE,
>> +};
>> +
>> +static int s32_gpio_get_ngpio(const struct s32_pinctrl_soc_data *soc_data,
>> + unsigned int *ngpio)
>> +{
>> + const struct s32_gpio_range *range;
>> + unsigned int end, max = 0;
>> + int i;
>> +
>> + if (!soc_data->gpio_ranges || !soc_data->num_gpio_ranges)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < soc_data->num_gpio_ranges; i++) {
>> + range = &soc_data->gpio_ranges[i];
>> +
>> + if (!range->gpio_num)
>> + return -EINVAL;
>> +
>> + end = range->gpio_base + range->gpio_num;
>> +
>> + /*
>> + * gpio_ranges must be ordered by gpio_base and must not overlap.
>> + * The GPIO line space size is derived from the highest range end.
>> + */
>> + if (i > 0 && range->gpio_base < max)
>> + return -EINVAL;
>> +
>> + if (end > max)
>> + max = end;
>> + }
>> +
>> + *ngpio = max;
>> +
>> + return 0;
>> +}
>> +
>> +static int s32_init_gpio_regmap(struct platform_device *pdev,
>> + struct s32_pinctrl *ipctl)
>> +{
>> + ipctl->gpio_regmap =
>> + devm_regmap_init(&pdev->dev, &s32_gpio_regmap_bus,
>> + ipctl, &s32_gpio_regmap_config);
>> + if (IS_ERR(ipctl->gpio_regmap))
>> + return dev_err_probe(&pdev->dev,
>> + PTR_ERR(ipctl->gpio_regmap),
>> + "Failed to init GPIO regmap\n");
>> +
>> + return 0;
>> +}
>> +
>> +static int s32_init_valid_mask(struct gpio_chip *chip, unsigned long *mask,
>> + unsigned int ngpios)
>> +{
>> + struct gpio_regmap *gpio = gpiochip_get_data(chip);
>> + struct s32_pinctrl *ipctl = gpio_regmap_get_drvdata(gpio);
>> + unsigned int gpio_num, pin, reg_offset;
>> + u16 pgpd_mask;
>> + int ret;
>> +
>> + bitmap_zero(mask, ngpios);
>> +
>> + for (gpio_num = 0; gpio_num < ngpios; gpio_num++) {
>> + ret = s32_gpio_get_range(ipctl, gpio_num, &pin, NULL);
>> + if (ret)
>> + continue;
>> +
>> + ret = s32_gpio_xlate_pgpd(ipctl, pin, ®_offset, &pgpd_mask);
>> + if (ret)
>> + continue;
>> +
>> + bitmap_set(mask, gpio_num, 1);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int s32_gpio_populate_names(struct s32_pinctrl *ipctl)
>> +{
>> + char **names;
>> + unsigned int gpio;
>> + unsigned int pin;
>> + char port;
>> + int ret;
>> +
>> + names = devm_kcalloc(ipctl->dev, ipctl->ngpio, sizeof(*names),
>> + GFP_KERNEL);
>> + if (!names)
>> + return -ENOMEM;
>> +
>> + for (gpio = 0; gpio < ipctl->ngpio; gpio++) {
>> + ret = s32_gpio_get_range(ipctl, gpio, &pin, NULL);
>> + if (ret)
>> + continue;
>> +
>> + port = 'A' + pin / 16;
>> +
>> + names[gpio] = devm_kasprintf(ipctl->dev, GFP_KERNEL,
>> + "P%c_%02u", port, pin & 0xf);
>> + if (!names[gpio])
>> + return -ENOMEM;
>> + }
>> +
>> + ipctl->gpio_names = (const char *const *)names;
>> +
>> + return 0;
>> +}
>> +
>> +static int s32_pinctrl_init_gpio_regmaps(struct platform_device *pdev,
>> + struct s32_pinctrl *ipctl)
>> +{
>> + const struct s32_pinctrl_soc_data *soc_data = ipctl->info->soc_data;
>> + static const struct regmap_config pgpd_config = {
>> + .reg_bits = 32,
>> + .val_bits = 16,
>> + .reg_stride = 2,
>> + };
>> + struct regmap_config cfg;
>> + struct resource *res;
>> + void __iomem *base;
>> + unsigned int pgpdo_idx, pgpdi_idx;
>> + unsigned int i;
>> +
>> + if (!soc_data->gpio_ranges || !soc_data->num_gpio_ranges)
>> + return 0;
>> +
>> + ipctl->num_gpio_regmaps = soc_data->num_gpio_ranges;
>> + ipctl->gpio_regmaps = devm_kcalloc(&pdev->dev, ipctl->num_gpio_regmaps,
>> + sizeof(*ipctl->gpio_regmaps),
>> + GFP_KERNEL);
>> + if (!ipctl->gpio_regmaps)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < ipctl->num_gpio_regmaps; i++) {
>> + ipctl->gpio_regmaps[i].range = &soc_data->gpio_ranges[i];
>> +
>> + /*
>> + * GPIO resources are placed after the pinctrl regions
>> + */
>> + pgpdo_idx = soc_data->mem_regions + i * 2;
>> + pgpdi_idx = soc_data->mem_regions + i * 2 + 1;
>> +
>> + /* PGPDO */
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, pgpdo_idx);
>> + if (!res)
>> + return dev_err_probe(&pdev->dev, -ENOENT,
>> + "Missing PGPDO resource %u\n", i);
>> +
>> + base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + cfg = pgpd_config;
>> + cfg.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "pgpdo%u", i);
>> + if (!cfg.name)
>> + return -ENOMEM;
>> +
>> + cfg.max_register = resource_size(res) - cfg.reg_stride;
>> +
>> + ipctl->gpio_regmaps[i].pgpdo =
>> + devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>> + if (IS_ERR(ipctl->gpio_regmaps[i].pgpdo))
>> + return dev_err_probe(&pdev->dev,
>> + PTR_ERR(ipctl->gpio_regmaps[i].pgpdo),
>> + "Failed to init PGPDO regmap %u\n", i);
>> +
>> + /* PGPDI */
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, pgpdi_idx);
>> + if (!res)
>> + return dev_err_probe(&pdev->dev, -ENOENT,
>> + "Missing PGPDI resource %u\n", i);
>> +
>> + base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + cfg = pgpd_config;
>> + cfg.name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "pgpdi%u", i);
>> + if (!cfg.name)
>> + return -ENOMEM;
>> +
>> + cfg.max_register = resource_size(res) - cfg.reg_stride;
>> +
>> + ipctl->gpio_regmaps[i].pgpdi =
>> + devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>> + if (IS_ERR(ipctl->gpio_regmaps[i].pgpdi))
>> + return dev_err_probe(&pdev->dev,
>> + PTR_ERR(ipctl->gpio_regmaps[i].pgpdi),
>> + "Failed to init PGPDI regmap %u\n", i);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> #ifdef CONFIG_PM_SLEEP
>> static bool s32_pinctrl_should_save(struct s32_pinctrl *ipctl,
>> unsigned int pin)
>> @@ -710,7 +1207,6 @@ int s32_pinctrl_suspend(struct device *dev)
>> const struct s32_pinctrl_soc_info *info = ipctl->info;
>> struct s32_pinctrl_context *saved_context = &ipctl->saved_context;
>> int i;
>> - int ret;
>> unsigned int config;
>>
>> for (i = 0; i < info->soc_data->npins; i++) {
>> @@ -719,9 +1215,7 @@ int s32_pinctrl_suspend(struct device *dev)
>> if (!s32_pinctrl_should_save(ipctl, pin->number))
>> continue;
>>
>> - ret = s32_regmap_read(ipctl->pctl, pin->number, &config);
>> - if (ret)
>> - return -EINVAL;
>> + s32_regmap_read(ipctl->pctl, pin->number, &config);
>>
>> saved_context->pads[i] = config;
>> }
>> @@ -736,7 +1230,7 @@ int s32_pinctrl_resume(struct device *dev)
>> const struct s32_pinctrl_soc_info *info = ipctl->info;
>> const struct pinctrl_pin_desc *pin;
>> struct s32_pinctrl_context *saved_context = &ipctl->saved_context;
>> - int ret, i;
>> + int i;
>>
>> for (i = 0; i < info->soc_data->npins; i++) {
>> pin = &info->soc_data->pins[i];
>> @@ -744,10 +1238,8 @@ int s32_pinctrl_resume(struct device *dev)
>> if (!s32_pinctrl_should_save(ipctl, pin->number))
>> continue;
>>
>> - ret = s32_regmap_write(ipctl->pctl, pin->number,
>> - saved_context->pads[i]);
>> - if (ret)
>> - return ret;
>> + s32_regmap_write(ipctl->pctl, pin->number,
>> + saved_context->pads[i]);
>> }
>>
>> return 0;
>> @@ -927,13 +1419,15 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
>> int s32_pinctrl_probe(struct platform_device *pdev,
>> const struct s32_pinctrl_soc_data *soc_data)
>> {
>> - struct s32_pinctrl *ipctl;
>> - int ret;
>> - struct pinctrl_desc *s32_pinctrl_desc;
>> - struct s32_pinctrl_soc_info *info;
>> #ifdef CONFIG_PM_SLEEP
>> struct s32_pinctrl_context *saved_context;
>> #endif
>> + struct gpio_regmap_config gpio_cfg = {};
>> + struct pinctrl_desc *s32_pinctrl_desc;
>> + struct s32_pinctrl_soc_info *info;
>> + struct s32_pinctrl *ipctl;
>> + unsigned int ngpio;
>> + int ret;
>>
>> if (!soc_data || !soc_data->pins || !soc_data->npins) {
>> dev_err(&pdev->dev, "wrong pinctrl info\n");
>> @@ -959,6 +1453,11 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>> INIT_LIST_HEAD(&ipctl->gpio_configs);
>> spin_lock_init(&ipctl->gpio_configs_lock);
>>
>> + ret = devm_add_action_or_reset(&pdev->dev,
>> + s32_gpio_free_saved_configs, ipctl);
>> + if (ret)
>> + return ret;
>> +
>> s32_pinctrl_desc =
>> devm_kzalloc(&pdev->dev, sizeof(*s32_pinctrl_desc), GFP_KERNEL);
>> if (!s32_pinctrl_desc)
>> @@ -978,6 +1477,11 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>> return ret;
>> }
>>
>> + ret = s32_pinctrl_init_gpio_regmaps(pdev, ipctl);
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret,
>> + "Failed to init GPIO regmaps\n");
>> +
>> ret = devm_pinctrl_register_and_init(&pdev->dev, s32_pinctrl_desc,
>> ipctl, &ipctl->pctl);
>> if (ret)
>> @@ -999,7 +1503,42 @@ int s32_pinctrl_probe(struct platform_device *pdev,
>> return dev_err_probe(&pdev->dev, ret,
>> "Failed to enable pinctrl\n");
>>
>> - dev_info(&pdev->dev, "Initialized S32 pinctrl driver\n");
>> + /* Setup GPIO if GPIO ranges are defined */
>> + if (!soc_data->gpio_ranges || !soc_data->num_gpio_ranges)
>> + return 0;
>> +
>> + ret = s32_gpio_get_ngpio(soc_data, &ngpio);
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret, "Invalid GPIO ranges\n");
>> +
>> + ipctl->ngpio = ngpio;
>> +
>> + ret = s32_gpio_populate_names(ipctl);
>> + if (ret)
>> + return ret;
>> +
>> + ret = s32_init_gpio_regmap(pdev, ipctl);
>> + if (ret)
>> + return ret;
>> +
>> + gpio_cfg.parent = &pdev->dev;
>> + gpio_cfg.fwnode = dev_fwnode(&pdev->dev);
>> + gpio_cfg.label = dev_name(&pdev->dev);
>> + gpio_cfg.regmap = ipctl->gpio_regmap;
>> + gpio_cfg.ngpio = ngpio;
>> + gpio_cfg.names = ipctl->gpio_names;
>> + gpio_cfg.reg_dir_out_base = GPIO_REGMAP_ADDR(S32_GPIO_OP_DIR);
>> + gpio_cfg.reg_dat_base = GPIO_REGMAP_ADDR(S32_GPIO_OP_DAT);
>> + gpio_cfg.reg_set_base = GPIO_REGMAP_ADDR(S32_GPIO_OP_SET);
>> + gpio_cfg.reg_mask_xlate = s32_gpio_reg_mask_xlate;
>> + gpio_cfg.init_valid_mask = s32_init_valid_mask;
>> + gpio_cfg.drvdata = ipctl;
>> +
>> + ipctl->gpio_rgm = devm_gpio_regmap_register(&pdev->dev, &gpio_cfg);
>> + if (IS_ERR(ipctl->gpio_rgm))
>> + return dev_err_probe(&pdev->dev,
>> + PTR_ERR(ipctl->gpio_rgm),
>> + "Unable to add gpio_regmap chip\n");
>>
>> return 0;
>> }
>> diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
>> index c49d28793b69..0bd6e6ab5ad1 100644
>> --- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
>> +++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
>> @@ -3,7 +3,7 @@
>> * NXP S32G pinctrl driver
>> *
>> * Copyright 2015-2016 Freescale Semiconductor, Inc.
>> - * Copyright 2017-2018, 2020-2022 NXP
>> + * Copyright 2017-2018, 2020-2022, 2025-2026 NXP
>> * Copyright (C) 2022 SUSE LLC
>> */
>>
>> @@ -773,17 +773,47 @@ static const struct s32_pin_range s32_pin_ranges_siul2[] = {
>> S32_PIN_RANGE(942, 1007),
>> };
>>
>> -static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
>> +static const struct s32_gpio_range s32_gpio_ranges_siul2[] = {
>> + S32_GPIO_RANGE(0, 0, 102),
>> + S32_GPIO_RANGE(112, 112, 79),
>> +};
>> +
>> +/*
>> + * SIUL2_1 GPIO ranges mapped to sparse PGPD pads.
>> + *
>> + * SIUL2_1 does not expose GPIO data registers as a linear pad
>> + * sequence. Each entry describes a contiguous GPIO offset range
>> + * and the PGPD pad servicing that range.
>> + */
>> +static const struct s32_gpio_pad_map s32g_gpio_pad_maps[] = {
>> + { 112, 122, 7 }, /* PH_00 .. PH_10 -> PGPD7 */
>> + { 144, 159, 9 }, /* PJ_00 .. PJ_15 -> PGPD9 */
>> + { 160, 175, 10 }, /* PK_00 .. PK_15 -> PGPD10 */
>> + { 176, 190, 11 }, /* PL_00 .. PL_14 -> PGPD11 */
>> +};
>> +
>> +/* Legacy data for old DT bindings without GPIO support */
>> +static const struct s32_pinctrl_soc_data legacy_s32g_pinctrl_data = {
>> + .pins = s32_pinctrl_pads_siul2,
>> + .npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
>> + .mem_pin_ranges = s32_pin_ranges_siul2,
>> + .mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
>> +};
>> +
>> +static const struct s32_pinctrl_soc_data s32g_pinctrl_data = {
>> .pins = s32_pinctrl_pads_siul2,
>> .npins = ARRAY_SIZE(s32_pinctrl_pads_siul2),
>> .mem_pin_ranges = s32_pin_ranges_siul2,
>> .mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
>> + .gpio_ranges = s32_gpio_ranges_siul2,
>> + .num_gpio_ranges = ARRAY_SIZE(s32_gpio_ranges_siul2),
>> + .gpio_pad_maps = s32g_gpio_pad_maps,
>> + .num_gpio_pad_maps = ARRAY_SIZE(s32g_gpio_pad_maps),
>> };
>>
>> static const struct of_device_id s32_pinctrl_of_match[] = {
>> {
>> .compatible = "nxp,s32g2-siul2-pinctrl",
>> - .data = &s32_pinctrl_data,
>> },
>> { /* sentinel */ }
>> };
>> @@ -792,8 +822,16 @@ MODULE_DEVICE_TABLE(of, s32_pinctrl_of_match);
>> static int s32g_pinctrl_probe(struct platform_device *pdev)
>> {
>> const struct s32_pinctrl_soc_data *soc_data;
>> + struct device_node *np = pdev->dev.of_node;
>>
>> - soc_data = of_device_get_match_data(&pdev->dev);
>> + /*
>> + * Legacy DTs only describe the pinctrl resources.
>> + * New DT changes extend the same node with GPIO resources.
>> + */
>> + if (of_property_present(np, "gpio-controller"))
>> + soc_data = &s32g_pinctrl_data;
>> + else
>> + soc_data = &legacy_s32g_pinctrl_data;
>>
>> return s32_pinctrl_probe(pdev, soc_data);
>> }
>> --
>> 2.34.1
>>
>
Hi Enric,
Thanks for your feedback!
I have addressed the two points from your review in
the new patch series (v11):
- `kmalloc_obj` usage is now correct in the GPIO request path
- The SIUL2_1 sparse PGPD layout is handled via a `sparse`
flag on `struct s32_gpio_range` and a pad-map-driven xlate,
removing the magic number 112 that was previously used
as a boundary check.
Regarding the remaining checkpatch warning:
I don t think `cfg` should be declared `const` because
it is a per-iteration copy of the `pgpd_config` template
that gets two fields written at runtime: `cfg.name` and
`cfg.max_register`(derived from the resource size).
Making the base template `pgpd_config` const (which it is)
I think is the right approach here.
Best regards,
Khristine
^ permalink raw reply
* Re: [PATCH v2 01/16] device property: Add fwnode_graph_get_port_by_id()
From: Andy Shevchenko @ 2026-06-10 13:57 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Bartosz Golaszewski, Greg Kroah-Hartman, Daniel Scally,
Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, AngeloGioacchino Del Regno, Alan Stern,
linux-acpi, driver-core, linux-pm, linux-usb, devicetree,
linux-mediatek, linux-arm-kernel, linux-kernel,
Manivannan Sadhasivam
In-Reply-To: <20260610084053.2059858-2-wenst@chromium.org>
On Wed, Jun 10, 2026 at 04:40:35PM +0800, Chen-Yu Tsai wrote:
> In some cases the driver needs a reference to the port firmware node.
> Once such case is the upcoming USB power sequencing integration. The
> USB hub port is tied to the corresponding port firmware node if it
> exists.
>
> Provide a helper for this.
Okay, if it's really needed.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
> +/**
> + * fwnode_graph_get_port_by_id - get the port matching a given id
> + * @fwnode: parent fwnode_handle containing the graph
> + * @id: id of the port
> + *
> + * Return: A 'port' firmware node pointer with refcount incremented.
> + *
> + * The caller is responsible for calling fwnode_handle_put() on the returned
> + * fwnode pointer.
Note, the Return section must be last one in the kernel-doc. The last paragraph
sounds to me as a better fit for main description. Basically check how other
kernel-doc(s) in this file are organised and follow that pattern.
> + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 04/12] clk: qcom: gcc-mdm9607: Fix BIMC PLL definition
From: Stephan Gerhold @ 2026-06-10 13:56 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Georgi Djakov,
Shawn Guo, Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <69cc948f-bb4f-49e7-b050-9e3805aef758@oss.qualcomm.com>
On Wed, Jun 10, 2026 at 03:46:00PM +0200, Konrad Dybcio wrote:
> On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> > From: Stephan Gerhold <stephan@gerhold.net>
> >
> > The gcc-mdm9607 driver was originally based on gcc-msm8916, but a closer
> > match nowadays is gcc-msm8909. Looking at the differences between
> > gcc-mdm9607 and gcc-msm8909, there is quite some confusion around the
> > definitions for the BIMC PLL.
> >
> > It turns out the BIMC PLL on MDM9607 is actually an Alpha PLL just like on
> > MSM8909. We can vote for it using BIT(2), which explains why BIT(3) was
> > used for GPLL2.
> >
> > Replace the definitions of the BIMC PLL in gcc-mdm9607 with the ones from
> > gcc-msm8909 to make sure we read the PLL status correctly and don't make
> > accidental incorrect changes to the PLL configuration. With this change,
> > the rate of the BIMC PLL (600.6 MHz) is displayed correctly in debugfs.
> >
> > The clock naming typically used for Alpha PLLs differs from the older vote
> > PLLs. There is no device tree upstream for MDM9607 yet, so we can still
> > safely change the dt-bindings. Nevertheless, by keeping the old ID values
> > and swapping the two entries we can preserve ABI compatibility for compiled
> > DTBs. Previously, BIMC_PLL_VOTE (71) was the final PLL clock to reference
> > when needed, this is now just BIMC_PLL. The previous parent BIMC_PLL (70)
> > is now the parent BIMC_PLL_EARLY clock.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
>
> I think we may be better off removing the NoC PLLs and defer controlling
> them to RPM, on all platforms that define them..
>
Well, this is not controlling them, it just allows reading them. And
that works pretty well, so I'm not sure why we would need to remove that
on platforms where it works?
Thanks,
Stephan
^ permalink raw reply
* Re: [PATCH v4 05/13] clk: qcom: dispcc-qcm2290: Move to the latest common qcom_cc_probe() model
From: Konrad Dybcio @ 2026-06-10 13:55 UTC (permalink / raw)
To: Imran Shaik, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Konrad Dybcio,
Loic Poulain, Brian Masney
Cc: Ajit Pandey, Taniya Das, Jagadeesh Kona, linux-arm-msm, linux-clk,
devicetree, linux-kernel
In-Reply-To: <20260604-shikra-dispcc-gpucc-v4-5-8204f1029311@oss.qualcomm.com>
On 6/4/26 7:26 AM, Imran Shaik wrote:
> Update the QCM2290 DISPCC driver to use the qcom_cc_probe() model by moving
> the critical clocks handling and PLL configurations from probe to the
> driver_data to align with the latest convention.
>
> Signed-off-by: Imran Shaik <imran.shaik@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v7 1/2] arm64: dts: qcom: sm8250: sort out Iris power domains
From: Konrad Dybcio @ 2026-06-10 13:52 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, Jonathan Marek,
Rafael J. Wysocki, Bryan O'Donoghue, Vikash Garodia,
Dikshita Agarwal, Mauro Carvalho Chehab, Stanimir Varbanov,
Abhinav Kumar, Hans Verkuil, Stefan Schmidt, Konrad Dybcio,
Bryan O'Donoghue, Dikshita Agarwal, Ulf Hansson,
linux-arm-msm, linux-clk, devicetree, linux-kernel, linux-pm,
linux-media, Mauro Carvalho Chehab
In-Reply-To: <cacfl4livqlkuk2c223hgapl45ojd6hsqg5sqqd7g5g2q57kk6@codcnqhfz3e6>
On 6/10/26 3:34 PM, Dmitry Baryshkov wrote:
> On Wed, Jun 10, 2026 at 02:24:24PM +0200, Konrad Dybcio wrote:
>> On 6/4/26 6:22 PM, Dmitry Baryshkov wrote:
>>> On SM8250 Iris core requires two power rails to function, MX (for PLLs)
>>> and MMCX (for everything else). The commit fa245b3f06cd ("arm64: dts:
>>> qcom: sm8250: Add venus DT node") added only MX power rail, but omitted
>>> MMCX voltage levels.
>>>
>>> Add MMCX domain to the Iris device node.
>>>
>>> Fixes: fa245b3f06cd ("arm64: dts: qcom: sm8250: Add venus DT node")
>>> Reported-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> ---
>>
>> [...]
>>
>>> opp-720000000 {
>>> opp-hz = /bits/ 64 <720000000>;
>>> - required-opps = <&rpmhpd_opp_low_svs>;
>>> + required-opps = <&rpmhpd_opp_svs>,
>>> + <&rpmhpd_opp_low_svs>;
>>
>> So the computer tells me low_svs would be enough for PLL0 to generate 720MHz
>>
>> Is there some transient dependency that bumps this to svs?
>>
>> Your changelog mentions you altered this in v6, but I don't see any related
>> discussion
>
> There are two sources of information. The "clocks plan" and the "pll
> info". For some reason, the clock plan doesn't reflect actual PLL
> requirements. See the info on the corresponding PLL type.
OK thanks, now I think I asked this already in the past..
still, both point to LOWSVS @ 720 for sm8250
Konrad
^ permalink raw reply
* Re: [PATCH v4 4/5] iio: adc: versal-sysmon: add threshold event support
From: Andy Shevchenko @ 2026-06-10 13:51 UTC (permalink / raw)
To: Erim, Salih
Cc: Jonathan Cameron, Andy Shevchenko, David Lechner, Nuno Sá,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Conall O'Griofa, Michal Simek, Guenter Roeck, Salih Erim,
linux-iio, devicetree, linux-kernel
In-Reply-To: <2b3b6577-d850-4102-a77c-8e0f82082a3b@amd.com>
On Wed, Jun 10, 2026 at 12:46:52PM +0100, Erim, Salih wrote:
> On 09/06/2026 18:31, Andy Shevchenko wrote:
> > On Sun, Jun 07, 2026 at 10:02:47PM +0100, Erim, Salih wrote:
> > > On 07/06/2026 08:31, Andy Shevchenko wrote:
> > > > On Sat, Jun 06, 2026 at 06:17:06AM +0100, Salih Erim wrote:
...
> > > > > + ret = regmap_read(sysmon->regmap, upper_off, &upper_reg);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + ret = regmap_read(sysmon->regmap, lower_off, &lower_reg);
> > > > > + if (ret)
> > > > > + return ret;
> > > >
> > > > Half of the IO accessors have no error checks, these do.
> > > > Why is this inconsistency?
> > >
> > > The regmap calls in sysmon_unmask_worker and sysmon_iio_irq
> > > have no error checks because they run in contexts where errors
> > > cannot be propagated (workqueue, hardirq). The init path checks
> > > errors because it can return them to the caller. Will add a
> > > comment explaining this.
> >
> > But the error checks may be used to modify flow inside the respective functions.
>
> Accepted. Will add error checks in sysmon_unmask_worker and
> sysmon_iio_irq to bail out early on regmap failure instead of
> operating on undefined data.
Same comment as per v5, up to you as failing MMIO on HW level is a sign of
something bigger.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 08/12] clk: qcom: gcc-mdm9607: Drop incorrect system_noc_bfdcd_clk_src
From: Konrad Dybcio @ 2026-06-10 13:50 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-8-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> This clock does not seem to exist on MDM9607. Reading/writing the registers
> always results in 0.
>
> Presumably, this definition was mistakenly copied from gcc-msm8916. On
> MSM8916, this root clock is used for multimedia subsystems (camera,
> display, video). MDM9607 has none of that, so this clock was probably
> omitted in the hardware.
>
> There are no users inside gcc-mdm9607, so we can just drop it.
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
This one's inexistent indeed
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v5 4/5] iio: adc: versal-sysmon: add threshold event support
From: Andy Shevchenko @ 2026-06-10 13:50 UTC (permalink / raw)
To: Erim, Salih
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <6378c39f-f368-4fd6-a51d-6fe057fd38c0@amd.com>
On Wed, Jun 10, 2026 at 01:10:05PM +0100, Erim, Salih wrote:
> On 09/06/2026 18:54, Andy Shevchenko wrote:
> > On Mon, Jun 08, 2026 at 07:38:00PM +0100, Salih Erim wrote:
...
> > > + /*
> > > + * regmap errors are not checked here because the worker and IRQ
> > > + * handler cannot propagate errors. The MMIO regmap uses fast_io
> > > + * with direct readl/writel which cannot fail.
> >
> > OK (but they can fail on HW level to the point of bus errors or so :).
>
> Accepted. Will add error checks to modify flow on failure
> instead of just documenting why they're absent.
> > > + */
I put smiley in the reply as I don't think it's practical. MMIO failing on HW
level is sign of something much bigger (usually) than just a random neutrino.
So, it's up to you, if you want to add error handling and skip, for example,
write-back of ISR.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 07/12] clk: qcom: gcc-mdm9607: Drop incorrect apss_tcu_clk_src
From: Konrad Dybcio @ 2026-06-10 13:50 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-7-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> This clock does not seem to exist on MDM9607. Reading/writing the registers
> always results in 0 or crashes. The math in the frequency table is also
> broken. GPLL2 on MDM9607 runs at 480 MHz, so:
>
> - F(155000000, P_GPLL2, 6, 0, 0), // 480 MHz/6 = 80 MHz, not 155 MHz
> - F(310000000, P_GPLL2, 3, 0, 0), // 480 MHz/3 = 160 MHz, not 310 MHz
>
> Presumably, this definition was mistakenly copied as-is from gcc-msm8916
> (which uses 930 MHz for GPLL2). There are no branch consumers of this root
> clock inside gcc-mdm9607 (notably, gcc_apss_tcu_clk has bimc_ddr_clk_src as
> parent instead of this clock), so we can just drop it.
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
It does, CMD_RCGR @ 0x37000
Interestingly, there's also an SMMU instance.. although downstream doesn't
seem bothered with it
Konrad
^ permalink raw reply
* Re: (subset) [PATCH V3 0/8] PCI: imx6: Integrate pwrctrl API and update device trees
From: Manivannan Sadhasivam @ 2026-06-10 13:48 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
lpieralisi, kwilczynski, bhelgaas, hongxing.zhu, l.stach,
Sherry Sun (OSS)
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
sherry.sun
In-Reply-To: <20260520084904.2424253-1-sherry.sun@oss.nxp.com>
On Wed, 20 May 2026 16:48:56 +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> This series integrates the PCI pwrctrl framework into the pci-imx6
> driver and updates i.MX EVK board device trees to support it.
>
> Patches 2-8 update device trees for i.MX EVK boards which maintained
> by NXP to move power supply properties from the PCIe controller node
> to the Root Port child node, which is required for pwrctrl framework.
> Affected boards:
> - i.MX6Q/DL SABRESD
> - i.MX6SX SDB
> - i.MX8MM EVK
> - i.MX8MP EVK
> - i.MX8MQ EVK
> - i.MX8DXL/QM/QXP EVK
> - i.MX95 15x15/19x19 EVK
>
> [...]
Applied, thanks!
[1/8] PCI: imx6: Integrate new pwrctrl API for pci-imx6
commit: 85c1fcfa740d4c737f5575fc7251883e54227a51
Best regards,
--
Manivannan Sadhasivam <mani@kernel.org>
^ permalink raw reply
* Re: [PATCH 06/12] clk: qcom: gcc-mdm9607: Increase delay for USB PHY reset
From: Konrad Dybcio @ 2026-06-10 13:46 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-6-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> To conform to the specifications of the USB PHY, the reset signal should be
> asserted for at least 10us. Guarantee that by increasing the delay for the
> USB2_HS_PHY_ONLY_BCR reset control similar to commit dcc6c9fb7128 ("clk:
> qcom: gcc-msm8909: Increase delay for USB PHY reset").
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
I'm still not sure if we need the extra 5, but one does not reset the USB
PHY repeatedly so it's not a huge loss..
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 04/12] clk: qcom: gcc-mdm9607: Fix BIMC PLL definition
From: Konrad Dybcio @ 2026-06-10 13:46 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-4-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> The gcc-mdm9607 driver was originally based on gcc-msm8916, but a closer
> match nowadays is gcc-msm8909. Looking at the differences between
> gcc-mdm9607 and gcc-msm8909, there is quite some confusion around the
> definitions for the BIMC PLL.
>
> It turns out the BIMC PLL on MDM9607 is actually an Alpha PLL just like on
> MSM8909. We can vote for it using BIT(2), which explains why BIT(3) was
> used for GPLL2.
>
> Replace the definitions of the BIMC PLL in gcc-mdm9607 with the ones from
> gcc-msm8909 to make sure we read the PLL status correctly and don't make
> accidental incorrect changes to the PLL configuration. With this change,
> the rate of the BIMC PLL (600.6 MHz) is displayed correctly in debugfs.
>
> The clock naming typically used for Alpha PLLs differs from the older vote
> PLLs. There is no device tree upstream for MDM9607 yet, so we can still
> safely change the dt-bindings. Nevertheless, by keeping the old ID values
> and swapping the two entries we can preserve ABI compatibility for compiled
> DTBs. Previously, BIMC_PLL_VOTE (71) was the final PLL clock to reference
> when needed, this is now just BIMC_PLL. The previous parent BIMC_PLL (70)
> is now the parent BIMC_PLL_EARLY clock.
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
I think we may be better off removing the NoC PLLs and defer controlling
them to RPM, on all platforms that define them..
Konrad
^ permalink raw reply
* Re: [PATCH 12/12] clk: qcom: gcc-mdm9607: Drop redundant register update during probe
From: Konrad Dybcio @ 2026-06-10 13:44 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-12-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> GPLL0 is pretty much guaranteed to be already on (and voted) during boot,
> since it's used by the CPU and also various other components such as UART.
> We also vote for this bit in the actual GPLL0 definition, which will be set
> as soon as any driver in the kernel requires using the GPLL0 clock.
>
> All in all, this makes separately voting for GPLL0 during probe redundant,
> especially because the "acpuclock" in the comment is a downstream construct
> that does not exist in upstream.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 11/12] dt-bindings: clock: qcom,gcc-mdm9607: Use proper address in example
From: Konrad Dybcio @ 2026-06-10 13:44 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-11-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> Given that this is a dedicated schema for qcom,gcc-mdm9607, we might as
> well use the correct memory addresses in the example. This does not
> affect the validation itself, but will reduce confusion for readers.
>
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 10/12] dt-bindings: clock: qcom,gcc-mdm9607: Add missing "clocks" property
From: Konrad Dybcio @ 2026-06-10 13:44 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-10-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> gcc-mdm9607.c uses "fw_name", so it requires specifying the "xo" and
> "sleep_clk" clock source in the device tree. For some reason, this was
> never documented in the dt-bindings. Nowadays, qcom,gcc-mdm9607 has a
> dedicated schema, so we can just add it to the properties without any
> additional conditionals.
>
> Fixes: 6faa7e4ddce6 ("dt-bindings: clock: Add MDM9607 GCC clock bindings")
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 05/12] clk: qcom: gcc-mdm9607: Fix halt_reg for gcc_apss_axi_clk
From: Konrad Dybcio @ 2026-06-10 13:43 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-5-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> gcc_apss_axi_clk specifies a halt_reg of 0x4601c, but this is already used
> by gcc_apss_ahb_clk. The correct value according to the downstream driver
> is 0x46020.
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v3 2/8] soc: qcom: Add support for QMI TMD cooling devices
From: Dmitry Baryshkov @ 2026-06-10 13:42 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Gaurav Kohli, Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Amit Kucheria,
Manivannan Sadhasivam, Konrad Dybcio, Kees Cook,
Gustavo A. R. Silva, cros-qcom-dts-watchers, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, linux-pm,
linux-hardening, Manaf Meethalavalappu Pallikunhi, Casey Connolly
In-Reply-To: <33fe758a-a733-4f27-8824-3dff398e1cd3@oss.qualcomm.com>
On Tue, Jun 09, 2026 at 02:08:57PM +0200, Daniel Lezcano wrote:
> On 6/9/26 13:30, Dmitry Baryshkov wrote:
> > On Tue, Jun 09, 2026 at 03:52:57PM +0530, Gaurav Kohli wrote:
> > > From: Casey Connolly <casey.connolly@linaro.org>
> > >
> > > Add a Qualcomm QMI Thermal Mitigation Device (TMD) to support thermal
> > > cooling devices backed by remote subsystems.
> > >
> > > On several Qualcomm platforms, remote processors (for example modem and
> > > CDSP) expose thermal mitigation controls through the TMD QMI service.
> > > Client drivers need a way to discover that service, map DT thermal
> > > mitigation endpoints to cooling devices, and forward cooling state
> > > updates to the remote subsystem.
> > >
> > > Co-developed-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
> > > Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
> > > Signed-off-by: Casey Connolly <casey.connolly@linaro.org>
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> >
> > Wrong SoB chain.
>
> I think Gaurav wanted to reflect the changes did a back and forth between
> us, so I ended up in the delivery path somehow. I guess adding
> Co-developped-by should fix the SoB but won't reflect Gaurav and Casey did
> actually most of the work. So I'm fine if we remove my SoB to fix the chain
Anyway, Gaurav's SoB should be the last one (and it should be preceeded
by the CdB tag)
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 03/12] clk: qcom: gcc-mdm9607: Fix enable_reg for gcc_blsp1_sleep_clk
From: Konrad Dybcio @ 2026-06-10 13:42 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-3-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
>
> MDM9607 is similar to MSM8909, where the GCC_BLSP1_SLEEP_CBCR register is
> read-only and only has the CLK_OFF bit to check if the clock is running.
> This is a shared vote clock, the correct way to enable it is to vote for
> BLSP1_SLEEP_CLK_ENA (BIT(9)) in GCC_APCS_CLOCK_BRANCH_ENA_VOTE (0x45004).
>
> Cc: stable@vger.kernel.org
> Fixes: 48b7253264ea ("clk: qcom: Add MDM9607 GCC driver")
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH 02/12] clk: qcom: gcc-msm8939: Fix enable_reg for gcc_blsp1_sleep_clk
From: Konrad Dybcio @ 2026-06-10 13:40 UTC (permalink / raw)
To: Stephan Gerhold, Bjorn Andersson
Cc: Michael Turquette, Stephen Boyd, Georgi Djakov, Shawn Guo,
Bryan O'Donoghue, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Taniya Das, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260609-qcom-clk-mdm9607-fixes-v1-2-5e9717faf842@linaro.org>
On 6/9/26 4:14 PM, Stephan Gerhold wrote:
> MSM8939 is similar to MSM8916, where the GCC_BLSP1_SLEEP_CBCR register is
> read-only and only has the CLK_OFF bit to check if the clock is running.
> This is a shared vote clock, the correct way to enable it is to vote for
> BLSP1_SLEEP_CLK_ENA (BIT(9)) in GCC_APCS_CLOCK_BRANCH_ENA_VOTE (0x45004).
>
> Cc: stable@vger.kernel.org
> Fixes: 1664014e4679 ("clk: qcom: gcc-msm8939: Add MSM8939 Generic Clock Controller")
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox