[PATCH RFC 4/4] drivers: of-thermal: notify framework when sensor is tripped
Notify all related thermal zones when the sensor reports a thermal trip. Signed-off-by: Lina Iyer --- drivers/thermal/of-thermal.c | 83 ++-- include/linux/thermal.h | 4 ++ 2 files changed, 84 insertions(+), 3 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 6fb2eeb5b6cf..9b42bfed78bf 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -41,12 +41,18 @@ struct __thermal_bind_params { * @ops: sensor driver ops * @tz_list: list of thermal zones for this sensor * @lock: lock sensor during operations + * @low_trip: sensor's low trip temp + * @high_trip: sensor's high trip temp + * @low_tz: the thermal zone whose low trip is used as @low_trip + * @high_tz: the thermal zone whose high trip is used as @high_trip */ struct thermal_sensor { void *sensor_data; const struct thermal_zone_of_device_ops *ops; struct list_head tz_list; struct mutex lock; + int low_trip, high_trip; + struct thermal_zone_device *low_tz, *high_tz; }; /** @@ -102,18 +108,26 @@ static void __of_thermal_agg_trip(struct thermal_sensor *sensor, int low, high; int max_lo = INT_MIN; int min_hi = INT_MAX; - struct thermal_zone_device *tz; + struct thermal_zone_device *tz, *lo_tz = NULL, *hi_tz = NULL; list_for_each_entry(tz, &sensor->tz_list, sensor_tzd) { thermal_zone_get_trip(tz, &low, &high); - if (low > max_lo) + if (low > max_lo) { max_lo = low; - if (high < min_hi) + lo_tz = tz; + } + if (high < min_hi) { min_hi = high; + hi_tz = tz; + } } *floor = max_lo; *ceil = min_hi; + sensor->low_trip = max_lo; + sensor->high_trip = min_hi; + sensor->low_tz = lo_tz; + sensor->high_tz = hi_tz; } static int of_thermal_set_trips(struct thermal_zone_device *tz, @@ -427,6 +441,69 @@ static struct thermal_zone_device_ops of_thermal_ops = { /*** sensor API ***/ +static void thermal_zone_of_get_trip(struct thermal_zone_device *tz, +int temp, int *low_trip, int *hi_trip) +{ + struct __thermal_zone *data = tz->devdata; + int low = INT_MIN; + int hi = INT_MAX; + int i; + + for (i = 0; i < data->ntrips; i++) { + int trip_temp = data->trips[i].temperature; + + if (trip_temp < temp && trip_temp > low) + *low_trip = i; + if (trip_temp > temp && trip_temp < hi) + *hi_trip = i; + } +} + +/** + * thermal_zone_of_sensor_notify - notify framework of a trip + * @tzd: the thermal zone device + * + * Sensor drivers may use this API to notify the thermal framework that the + * temperature has crossed the trip threshold. This function is akin to + * thermal_notify_framework() call, but is expected to be called by a sensor + * that registered itself with the framework using the + * thermal_zone_of_add_sensor() function. + */ +void thermal_zone_of_sensor_notify(struct thermal_zone_device *tzd) +{ + struct __thermal_zone *tz = tzd->devdata; + struct thermal_sensor *sensor = tz->sensor; + int temp, low_trip, hi_trip, *trip; + struct thermal_zone_device *trip_tz = NULL; + + if (!tz->sensor) + return; + + if (of_thermal_get_temp(tzd, &temp)) + return; + + mutex_lock(&sensor->lock); + if (tzd->temperature < sensor->low_trip && sensor->low_tz) { + trip_tz = sensor->low_tz; + trip = &low_trip; + } + if (tzd->temperature > sensor->high_trip && sensor->high_tz) { + trip_tz = sensor->high_tz; + trip = &hi_trip; + } + + if (trip_tz) + thermal_zone_of_get_trip(trip_tz, temp, &low_trip, &hi_trip); + + mutex_unlock(&sensor->lock); + + if (!trip_tz) + return; + + thermal_notify_framework(trip_tz, *trip); +} +EXPORT_SYMBOL(thermal_zone_of_sensor_notify); + static struct thermal_zone_device * thermal_zone_of_add_sensor(struct device_node *zone, struct device_node *sensor_np, diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 000ae6a97678..603bf8065d7d 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -388,6 +388,7 @@ struct thermal_zone_device *devm_thermal_zone_of_sensor_register( const struct thermal_zone_of_device_ops *ops); void devm_thermal_zone_of_sensor_unregister(struct device *dev,
[PATCH RFC 3/4] drivers: of-thermal: aggregate sensor trips across thermal zones
Aggregate thermal trip based on the trip points of the thermal zones that use this sensor as the source. Signed-off-by: Lina Iyer --- drivers/thermal/of-thermal.c | 28 ++- drivers/thermal/thermal_helpers.c | 37 ++- include/linux/thermal.h | 4 3 files changed, 53 insertions(+), 16 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index 06e581a37c67..6fb2eeb5b6cf 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -96,16 +96,42 @@ static int of_thermal_get_temp(struct thermal_zone_device *tz, return sensor->ops->get_temp(sensor->sensor_data, temp); } +static void __of_thermal_agg_trip(struct thermal_sensor *sensor, +int *floor, int *ceil) +{ + int low, high; + int max_lo = INT_MIN; + int min_hi = INT_MAX; + struct thermal_zone_device *tz; + + list_for_each_entry(tz, &sensor->tz_list, sensor_tzd) { + thermal_zone_get_trip(tz, &low, &high); + if (low > max_lo) + max_lo = low; + if (high < min_hi) + min_hi = high; + } + + *floor = max_lo; + *ceil = min_hi; +} + static int of_thermal_set_trips(struct thermal_zone_device *tz, int low, int high) { struct __thermal_zone *data = tz->devdata; struct thermal_sensor *sensor = data->sensor; + int ret; if (!sensor || !sensor->ops->set_trips) return -EINVAL; - return sensor->ops->set_trips(sensor->sensor_data, low, high); + mutex_lock(&sensor->lock); + __of_thermal_agg_trip(sensor, &low, &high); + ret = sensor->ops->set_trips(sensor->sensor_data, low, high); + mutex_unlock(&sensor->lock); + + return ret; } /** diff --git a/drivers/thermal/thermal_helpers.c b/drivers/thermal/thermal_helpers.c index f550fdee0f9b..d4fa125f8799 100644 --- a/drivers/thermal/thermal_helpers.c +++ b/drivers/thermal/thermal_helpers.c @@ -113,32 +113,39 @@ int thermal_zone_get_temp(struct thermal_zone_device *tz, int *temp) } EXPORT_SYMBOL_GPL(thermal_zone_get_temp); -void thermal_zone_set_trips(struct thermal_zone_device *tz) + +void thermal_zone_get_trip(struct thermal_zone_device *tz, int *low, int *high) { - int low = -INT_MAX; - int high = INT_MAX; - int trip_temp, hysteresis; - int i, ret; + int i, trip_low, trip_temp, hysteresis; - mutex_lock(&tz->lock); - - if (!tz->ops->set_trips || !tz->ops->get_trip_hyst) - goto exit; + *low = -INT_MAX; + *high = INT_MAX; for (i = 0; i < tz->trips; i++) { - int trip_low; - tz->ops->get_trip_temp(tz, i, &trip_temp); tz->ops->get_trip_hyst(tz, i, &hysteresis); trip_low = trip_temp - hysteresis; - if (trip_low < tz->temperature && trip_low > low) - low = trip_low; + if (trip_low < tz->temperature && trip_low > *low) + *low = trip_low; - if (trip_temp > tz->temperature && trip_temp < high) - high = trip_temp; + if (trip_temp > tz->temperature && trip_temp < *high) + *high = trip_temp; } +} + +void thermal_zone_set_trips(struct thermal_zone_device *tz) +{ + int low, high; + int ret; + + mutex_lock(&tz->lock); + + if (!tz->ops->set_trips || !tz->ops->get_trip_hyst) + goto exit; + + thermal_zone_get_trip(tz, &low, &high); /* No need to change trip points */ if (tz->prev_low_trip == low && tz->prev_high_trip == high) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 09e1669a4919..000ae6a97678 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -443,6 +443,7 @@ int thermal_zone_unbind_cooling_device(struct thermal_zone_device *, int, struct thermal_cooling_device *); void thermal_zone_device_update(struct thermal_zone_device *, enum thermal_notify_event); +void thermal_zone_get_trip(struct thermal_zone_device *tz, int *low, int *high); void thermal_zone_set_trips(struct thermal_zone_device *); struct thermal_cooling_device *thermal_cooling_device_register(char *, void *, @@ -496,6 +497,9 @@ static inline int thermal_zone_unbind_cooling_device( static inline void thermal_zone_device_update(struct thermal_zone_device *tz, enum thermal_notify_event event) { } +stati
[PATCH RFC 2/4] drivers: of-thermal: allow multiple thermal zones for a sensor
To allow different mitigative actions based on a sensor temperature, it is desirable to have multiple thermal zones share the same source. The thermal zones could have different thresholds, mitigative actions and even different governors. Signed-off-by: Lina Iyer --- drivers/thermal/of-thermal.c | 32 ++-- include/linux/thermal.h | 2 ++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c index b402edd09ed1..06e581a37c67 100644 --- a/drivers/thermal/of-thermal.c +++ b/drivers/thermal/of-thermal.c @@ -39,10 +39,14 @@ struct __thermal_bind_params { * struct thermal_sensor - Holds individual sensor data * @sensor_data: sensor driver private data passed as input argument * @ops: sensor driver ops + * @tz_list: list of thermal zones for this sensor + * @lock: lock sensor during operations */ struct thermal_sensor { void *sensor_data; const struct thermal_zone_of_device_ops *ops; + struct list_head tz_list; + struct mutex lock; }; /** @@ -430,11 +434,24 @@ thermal_zone_of_add_sensor(struct device_node *zone, if (sensor->ops->set_emul_temp) tzd->ops->set_emul_temp = of_thermal_set_emul_temp; + mutex_lock(&sensor->lock); + list_add_tail(&tzd->sensor_tzd, &sensor->tz_list); + mutex_unlock(&sensor->lock); mutex_unlock(&tzd->lock); return tzd; } +static struct thermal_zone_device * +thermal_zone_of_get_sensor_tzd(struct thermal_sensor *sensor) +{ + if (list_empty(&sensor->tz_list)) + return ERR_PTR(-ENODEV); + + return list_first_entry(&sensor->tz_list, struct thermal_zone_device, + sensor_tzd); +} + /** * thermal_zone_of_sensor_register - registers a sensor to a DT thermal zone * @dev: a valid struct device pointer of a sensor device. Must contain @@ -460,12 +477,11 @@ thermal_zone_of_add_sensor(struct device_node *zone, * 01 - This function must enqueue the new sensor instead of using * it as the only source of temperature values. * - * 02 - There must be a way to match the sensor with all thermal zones - * that refer to it. - * * Return: On success returns a valid struct thermal_zone_device, * otherwise, it returns a corresponding ERR_PTR(). Caller must * check the return value with help of IS_ERR() helper. + * The returned thermal_zone_device is one of the thermal zones that is + * using this sensor as the temeprature source. */ struct thermal_zone_device * thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data, @@ -489,6 +505,8 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data, of_node_put(np); return ERR_PTR(-ENOMEM); } + INIT_LIST_HEAD(&sensor->tz_list); + mutex_init(&sensor->lock); sensor->sensor_data = data; sensor->ops = ops; sensor_np = of_node_get(dev->of_node); @@ -521,14 +539,14 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data, of_node_put(sensor_specs.np); of_node_put(child); - goto exit; } of_node_put(sensor_specs.np); } -exit: + of_node_put(sensor_np); of_node_put(np); + tzd = thermal_zone_of_get_sensor_tzd(sensor); if (tzd == ERR_PTR(-ENODEV)) kfree(sensor); return tzd; @@ -569,7 +587,9 @@ void thermal_zone_of_sensor_unregister(struct device *dev, tzd->ops->get_trend = NULL; tzd->ops->set_emul_temp = NULL; - kfree(tz->sensor); + list_del(&tzd->sensor_tzd); + if (list_empty(&tz->sensor->tz_list)) + kfree(tz->sensor); tz->sensor = NULL; mutex_unlock(&tzd->lock); } diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 7a133bd6ff86..09e1669a4919 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -188,6 +188,7 @@ struct thermal_attr { * @node: node in thermal_tz_list (in thermal_core.c) * @poll_queue:delayed work for polling * @notify_event: Last notification event + * @sensor_tzd:node in sensor's list of thermal zone devices */ struct thermal_zone_device { int id; @@ -220,6 +221,7 @@ struct thermal_zone_device { struct list_head node; struct delayed_work poll_queue; enum thermal_notify_event notify_event; + struct list_head sensor_tzd; }; /** -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] soc: qcom: cmd-db: Make endian-agnostic
On Thu, Apr 12 2018 at 16:41 -0600, Stephen Boyd wrote: This driver deals with memory that is stored in little-endian format. Update the structures with the proper little-endian types and then do the proper conversions when reading the fields. Note that we compare the ids with a memcmp() because we already pad out the string 'id' field to exactly 8 bytes with the strncpy() onto the stack. Cc: Mahesh Sivasubramanian Cc: Lina Iyer Cc: Bjorn Andersson Cc: Evan Green Signed-off-by: Stephen Boyd --- Changes from inline patch: * Fixed magic * Made function for memcmp() * drivers/soc/qcom/cmd-db.c | 116 +- 1 file changed, 65 insertions(+), 51 deletions(-) @@ -99,8 +90,34 @@ struct cmd_db_header { * h/w accelerator and request a resource state. */ +static const char CMD_DB_MAGIC[] = { 0xdb, 0x30, 0x03, 0x0c }; + This works now. +static bool cmd_db_magic_matches(struct cmd_db_header *header) +{ + __le32 *magic = &header->magic_num; + + return memcmp(magic, CMD_DB_MAGIC, sizeof(CMD_DB_MAGIC)) == 0; +} + static struct cmd_db_header *cmd_db_header; Could you move this up, along with other declarations? + +static inline void *rsc_to_entry_header(struct rsc_hdr *hdr) +{ + u16 offset = le16_to_cpu(hdr->header_offset); + + return cmd_db_header->data + offset; I noticed that the data was incorrect, with the above and I did it to read the correct data. return ((void*)cmd_db_header + sizeof(*cmd_db_header) + offset); +} + +static inline void * +rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent) +{ + u16 offset = le16_to_cpu(hdr->header_offset); + u16 loffset = le16_to_cpu(ent->offset); + + return cmd_db_header->data + offset + loffset; As well here - return ((void*)cmd_db_header + sizeof(*cmd_db_header) + offset + loffset); +} + Thanks, Lina
Re: [PATCH] soc: qcom: cmd-db: Make endian-agnostic
On Tue, Apr 17 2018 at 11:22 -0600, Lina Iyer wrote: On Thu, Apr 12 2018 at 16:41 -0600, Stephen Boyd wrote: This driver deals with memory that is stored in little-endian format. Update the structures with the proper little-endian types and then do the proper conversions when reading the fields. Note that we compare the ids with a memcmp() because we already pad out the string 'id' field to exactly 8 bytes with the strncpy() onto the stack. Cc: Mahesh Sivasubramanian Cc: Lina Iyer Cc: Bjorn Andersson Cc: Evan Green Signed-off-by: Stephen Boyd --- Changes from inline patch: * Fixed magic * Made function for memcmp() * drivers/soc/qcom/cmd-db.c | 116 +- 1 file changed, 65 insertions(+), 51 deletions(-) @@ -99,8 +90,34 @@ struct cmd_db_header { * h/w accelerator and request a resource state. */ +static const char CMD_DB_MAGIC[] = { 0xdb, 0x30, 0x03, 0x0c }; + This works now. +static bool cmd_db_magic_matches(struct cmd_db_header *header) +{ + __le32 *magic = &header->magic_num; + + return memcmp(magic, CMD_DB_MAGIC, sizeof(CMD_DB_MAGIC)) == 0; +} + static struct cmd_db_header *cmd_db_header; Could you move this up, along with other declarations? + +static inline void *rsc_to_entry_header(struct rsc_hdr *hdr) +{ + u16 offset = le16_to_cpu(hdr->header_offset); + + return cmd_db_header->data + offset; I noticed that the data was incorrect, with the above and I did it to read the correct data. Spoke too soon. Let me test more and report. -- Lina return ((void*)cmd_db_header + sizeof(*cmd_db_header) + offset); +} + +static inline void * +rsc_offset(struct rsc_hdr *hdr, struct entry_header *ent) +{ + u16 offset = le16_to_cpu(hdr->header_offset); + u16 loffset = le16_to_cpu(ent->offset); + + return cmd_db_header->data + offset + loffset; As well here - return ((void*)cmd_db_header + sizeof(*cmd_db_header) + offset + loffset); +} + Thanks, Lina
Re: [PATCH v5 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs
On Mon, Apr 16 2018 at 00:01 -0600, Stephen Boyd wrote: Quoting Lina Iyer (2018-04-16 09:08:18) On Fri, Apr 13 2018 at 16:40 -0600, Stephen Boyd wrote: >Well it seems like an RSC contains many DRVs and those DRVs contain many >TCSes. This is what I get after talking with Bjorn on IRC. > > +--+ (0x0) > | | > | DRV #0 | > | | > |-- --| (tcs-offset (0xd00)) > | DRV0_TCS0 | > |common space | > |cmd sequencer | 0xd00 + 0x14 > | | > | DRV0_TCS1 | > |common space | 0xd00 + 0x2a0 > |cmd sequencer | 0xd00 + 0x2a0 + 0x14 > | | > | DRV0_TCS2 | > | | > | | > +--+ (0x1) > | | > | DRV #1 | > | | > |-- --| (tcs-offset) > | DRV1_TCS0 | > | DRV1_TCS1 | > | DRV1_TCS2 | > +--+ (0x2) > | | > | DRV #2 | > | | > |-- --| > | DRV2_TCS0 | > | DRV2_TCS1 | > | DRV2_TCS2 | > | DRV2_TCS3 | > | DRV2_TCS4 | > | DRV2_TCS5 | > +--+ > >I think I understand it now. There aren't any "RSC common" registers >that are common to the entire RSC. Instead, everything goes into a DRV, >or into a common TCS space, or into a TCS "queue". > >> >Put another way, even if the "apps" RSC is complicated, we should be >> >describing it to the best of our abilities in the binding so that when >> >it is used by non-linux OSes things still work by simply tweaking the >> >drv-id that we use to pick the right things out of the node. >> > >> >Or we're describing the RSC but it's really a container node that >> >doesn't do much besides hold DRVs? So this is described at the wrong >> >level? >> What we are describing is a DRV, but a standalone DRV alone is useless >> without the necessary RSC registers. So its a unique RSC+DRV combination >> that is represented here. >> > >If my understanding is correct up there then the binding could either >describe a single RSC DRV, or it could describe all the RSC DRV >instances and interrupts going into the RSC "block" and then we can use >drv-id to pick the offset we jump to. > Your understanding is correct. >I imagine we don't have any practical use-case for the entire RSC space >because there aren't any common RSC registers to deal with. Not true. So then my understanding is not correct! :/ >So we've >boiled this all down to describing one DRV and then I wonder why we care >about having drv-id at all? It looks to be used to check for a max >number of TCS, but that's already described by DT so it doesn't seem >very useful to double check what the hardware can tells us. > There is also a number of commands per TCS (NCPT), that may way vary between different RSCs. The RSC of the application processor has 16 commands in each TCS, but that is variable. I am not saying it cannot be described in DT, but it is something I read from the common RSC registers, currently. Also, I will using common/DRV0 registers to write wakeup time value, when the processor subsystem goes into power down. This is not DRV2 register, but is a
Re: linux-next: manual merge of the pm tree with the imx-mxs tree
On Tue, Oct 25 2016 at 17:51 -0600, Rafael J. Wysocki wrote: On Tuesday, October 25, 2016 10:47:29 AM Stephen Rothwell wrote: Hi Rafael, Today's linux-next merge of the pm tree got a conflict in: arch/arm/mach-imx/gpc.c between commits: eef0b282bb58 ("ARM: imx: gpc: Initialize all power domains") f9d1f7a7ad91 ("ARM: imx: gpc: Fix the imx_gpc_genpd_init() error path") from the imx-mxs tree and commit: 59d65b73a23c ("PM / Domains: Make genpd state allocation dynamic") from the pm tree. I fixed it up (see below) and can carry the fix as necessary. Thanks Stephen! Lina, please have a look at the Stephen's fix and let me know if that conflict should be resolved in a different way. Hi Rafael, Stephen's conflict resolution seems correct. Thanks Stephen. -- Lina
Re: [PATCH RFC v2 0/2] hwspinlock: Introduce raw capability for hwspinlock_device
Hi Ohad, Any comments? Thanks, Lina On Tue, Jun 09 2015 at 10:23 -0600, Lina Iyer wrote: This patch follows the discussion based on the first RFC series posted on the mailing list [1]. The discussion resulted in a couple of directives for hwspinlocks that do not want the framework imposing a s/w spinlock around the hwspinlock. i. The default should only be a s/w spinlock around the hwspinlock to ensure correctness of locking. ii. Existing users may not use raw capability, unless the platform registers support for it. iii. Platform driver for hwspinlock should dictate which locks can be operated in raw mode. iv. Platform driver and the hw holds the responsibility to ensure the correctness of acquiring the hwspinlock. This patchset implements these directives. Changes since RFC v1: - Introduce 'raw' capability for hwspinlocks. - Platform code now has to explicitly specify the raw capability of a lock. - Check to ensure that only those locks explicitly marked as raw capable can be locked/unlocked through the _raw api - QCOM patch for making lock #7 raw capable added. - Add documentation Thanks, Lina [1]. https://patches.linaro.org/47895/ Lina Iyer (2): hwspinlock: Introduce raw capability for hwspinlocks hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id Documentation/hwspinlock.txt | 16 +++ drivers/hwspinlock/hwspinlock_core.c | 75 +++- drivers/hwspinlock/hwspinlock_internal.h | 6 +++ drivers/hwspinlock/qcom_hwspinlock.c | 22 +++--- include/linux/hwspinlock.h | 41 + 5 files changed, 125 insertions(+), 35 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 2/3] ARM: cpuidle: refine cpuidle_ops member's parameters.
On Thu, Jul 09 2015 at 03:28 -0600, Lorenzo Pieralisi wrote: On Thu, Jul 09, 2015 at 09:43:04AM +0100, Jisheng Zhang wrote: On Thu, 9 Jul 2015 16:31:24 +0800 Jisheng Zhang wrote: > As for the suspend member function, the to-be-suspended cpu is always > the calling cpu itself, so the 'cpu' parameter may not be necessary, let > driver get 'cpu' itself if need. > > As for the init member function, the device_node here may not be > necessary either, because we can get the node via. of_get_cpu_node(). This patch also changes drivers/soc/qcom/spm.c accordingly, but I have no qcom platform, so I can't test it, just make sure it can pass kernel building. Could anyone kindly help me to test? CC'ing Lina who should be able to help you test it. Sure, will test and let you know. -- Lina Thanks, Lorenzo Thanks a lot, Jisheng > > Signed-off-by: Jisheng Zhang > --- > arch/arm/include/asm/cpuidle.h | 6 +++--- > arch/arm/kernel/cpuidle.c | 8 > drivers/soc/qcom/spm.c | 17 - > 3 files changed, 19 insertions(+), 12 deletions(-) > > diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h > index 0f84249..ca5dfe6 100644 > --- a/arch/arm/include/asm/cpuidle.h > +++ b/arch/arm/include/asm/cpuidle.h > @@ -30,8 +30,8 @@ static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev, > struct device_node; > > struct cpuidle_ops { > - int (*suspend)(int cpu, unsigned long arg); > - int (*init)(struct device_node *, int cpu); > + int (*suspend)(unsigned long arg); > + int (*init)(unsigned int cpu); > }; > > struct of_cpuidle_method { > @@ -46,6 +46,6 @@ struct of_cpuidle_method { > > extern int arm_cpuidle_suspend(int index); > > -extern int arm_cpuidle_init(int cpu); > +extern int arm_cpuidle_init(unsigned int cpu); > > #endif > diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c > index 318da33..7bc7bc6 100644 > --- a/arch/arm/kernel/cpuidle.c > +++ b/arch/arm/kernel/cpuidle.c > @@ -53,10 +53,10 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev, > int arm_cpuidle_suspend(int index) > { >int ret = -EOPNOTSUPP; > - int cpu = smp_processor_id(); > + unsigned int cpu = smp_processor_id(); > >if (cpuidle_ops[cpu].suspend) > - ret = cpuidle_ops[cpu].suspend(cpu, index); > + ret = cpuidle_ops[cpu].suspend(index); > >return ret; > } > @@ -134,7 +134,7 @@ static int __init arm_cpuidle_read_ops(struct device_node *dn, int cpu) > * -ENXIO if the HW reports a failure or a misconfiguration, > * -ENOMEM if the HW report an memory allocation failure > */ > -int __init arm_cpuidle_init(int cpu) > +int __init arm_cpuidle_init(unsigned int cpu) > { >struct device_node *cpu_node = of_cpu_device_node_get(cpu); >int ret; > @@ -144,7 +144,7 @@ int __init arm_cpuidle_init(int cpu) > >ret = arm_cpuidle_read_ops(cpu_node, cpu); >if (!ret && cpuidle_ops[cpu].init) > - ret = cpuidle_ops[cpu].init(cpu_node, cpu); > + ret = cpuidle_ops[cpu].init(cpu); > >of_node_put(cpu_node); > > diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c > index b04b05a..1649ec3 100644 > --- a/drivers/soc/qcom/spm.c > +++ b/drivers/soc/qcom/spm.c > @@ -116,7 +116,7 @@ static const struct spm_reg_data spm_reg_8064_cpu = { > > static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv); > > -typedef int (*idle_fn)(int); > +typedef int (*idle_fn)(void); > static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops); > > static inline void spm_register_write(struct spm_driver_data *drv, > @@ -179,9 +179,10 @@ static int qcom_pm_collapse(unsigned long int unused) >return -1; > } > > -static int qcom_cpu_spc(int cpu) > +static int qcom_cpu_spc(void) > { >int ret; > + int cpu = smp_processor_id(); >struct spm_driver_data *drv = per_cpu(cpu_spm_drv, cpu); > >spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC); > @@ -197,9 +198,11 @@ static int qcom_cpu_spc(int cpu) >return ret; > } > > -static int qcom_idle_enter(int cpu, unsigned long index) > +static int qcom_idle_enter(unsigned long index) > { > - return per_cpu(qcom_idle_ops, cpu)[index](cpu); > + unsigned int cpu = smp_processor_id(); > + > + return per_cpu(qcom_idle_ops, cpu)[index](); > } > > static const struct of_device_id qcom_idle_state_match[] __initconst = { > @@ -207,7 +210,7 @@ static const struct of_device_id qcom_idle_state_match[] __initconst = { >{ }, > }; > > -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu) > +static int __init qcom_cpuidle_init(unsigned int cpu) > { >const struct of_device_id *match_id; >struct device_node *state_node; > @@ -217,6 +220,10 @@ static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu) >idle_fn *fns; >cpumask_t mask; >bool use_scm_power_down = false; > + struct device_node *cpu_node = of_get_cpu_node(cpu, NULL); > + > + if (!cpu_node) > + return -ENODEV; > >for (i = 0; ; i++) { >
Re: [PATCH v3 2/3] ARM: cpuidle: refine cpuidle_ops member's parameters.
On Thu, Jul 09 2015 at 02:33 -0600, Jisheng Zhang wrote: As for the suspend member function, the to-be-suspended cpu is always the calling cpu itself, so the 'cpu' parameter may not be necessary, let driver get 'cpu' itself if need. As for the init member function, the device_node here may not be necessary either, because we can get the node via. of_get_cpu_node(). Signed-off-by: Jisheng Zhang Tested-by: Lina Iyer --- arch/arm/include/asm/cpuidle.h | 6 +++--- arch/arm/kernel/cpuidle.c | 8 drivers/soc/qcom/spm.c | 17 - 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h index 0f84249..ca5dfe6 100644 --- a/arch/arm/include/asm/cpuidle.h +++ b/arch/arm/include/asm/cpuidle.h @@ -30,8 +30,8 @@ static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev, struct device_node; struct cpuidle_ops { - int (*suspend)(int cpu, unsigned long arg); - int (*init)(struct device_node *, int cpu); + int (*suspend)(unsigned long arg); + int (*init)(unsigned int cpu); }; struct of_cpuidle_method { @@ -46,6 +46,6 @@ struct of_cpuidle_method { extern int arm_cpuidle_suspend(int index); -extern int arm_cpuidle_init(int cpu); +extern int arm_cpuidle_init(unsigned int cpu); #endif diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c index 318da33..7bc7bc6 100644 --- a/arch/arm/kernel/cpuidle.c +++ b/arch/arm/kernel/cpuidle.c @@ -53,10 +53,10 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev, int arm_cpuidle_suspend(int index) { int ret = -EOPNOTSUPP; - int cpu = smp_processor_id(); + unsigned int cpu = smp_processor_id(); if (cpuidle_ops[cpu].suspend) - ret = cpuidle_ops[cpu].suspend(cpu, index); + ret = cpuidle_ops[cpu].suspend(index); return ret; } @@ -134,7 +134,7 @@ static int __init arm_cpuidle_read_ops(struct device_node *dn, int cpu) * -ENXIO if the HW reports a failure or a misconfiguration, * -ENOMEM if the HW report an memory allocation failure */ -int __init arm_cpuidle_init(int cpu) +int __init arm_cpuidle_init(unsigned int cpu) { struct device_node *cpu_node = of_cpu_device_node_get(cpu); int ret; @@ -144,7 +144,7 @@ int __init arm_cpuidle_init(int cpu) ret = arm_cpuidle_read_ops(cpu_node, cpu); if (!ret && cpuidle_ops[cpu].init) - ret = cpuidle_ops[cpu].init(cpu_node, cpu); + ret = cpuidle_ops[cpu].init(cpu); of_node_put(cpu_node); diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c index b04b05a..1649ec3 100644 --- a/drivers/soc/qcom/spm.c +++ b/drivers/soc/qcom/spm.c @@ -116,7 +116,7 @@ static const struct spm_reg_data spm_reg_8064_cpu = { static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv); -typedef int (*idle_fn)(int); +typedef int (*idle_fn)(void); static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops); static inline void spm_register_write(struct spm_driver_data *drv, @@ -179,9 +179,10 @@ static int qcom_pm_collapse(unsigned long int unused) return -1; } -static int qcom_cpu_spc(int cpu) +static int qcom_cpu_spc(void) { int ret; + int cpu = smp_processor_id(); struct spm_driver_data *drv = per_cpu(cpu_spm_drv, cpu); spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC); @@ -197,9 +198,11 @@ static int qcom_cpu_spc(int cpu) return ret; } -static int qcom_idle_enter(int cpu, unsigned long index) +static int qcom_idle_enter(unsigned long index) { - return per_cpu(qcom_idle_ops, cpu)[index](cpu); + unsigned int cpu = smp_processor_id(); + + return per_cpu(qcom_idle_ops, cpu)[index](); } static const struct of_device_id qcom_idle_state_match[] __initconst = { @@ -207,7 +210,7 @@ static const struct of_device_id qcom_idle_state_match[] __initconst = { { }, }; -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu) +static int __init qcom_cpuidle_init(unsigned int cpu) { const struct of_device_id *match_id; struct device_node *state_node; @@ -217,6 +220,10 @@ static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu) idle_fn *fns; cpumask_t mask; bool use_scm_power_down = false; + struct device_node *cpu_node = of_get_cpu_node(cpu, NULL); + + if (!cpu_node) + return -ENODEV; for (i = 0; ; i++) { state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i); -- 2.1.4 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers: qcom: Select QCOM_SCM unconditionally for QCOM_PM
Enable QCOM_SCM for QCOM power management driver Signed-off-by: Lina Iyer --- drivers/soc/qcom/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 5eea374..e9a2c19 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -13,6 +13,7 @@ config QCOM_GSBI config QCOM_PM bool "Qualcomm Power Management" depends on ARCH_QCOM && !ARM64 + select QCOM_SCM help QCOM Platform specific power driver to manage cores and L2 low power modes. It interface with various system drivers to put the cores in -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Build error with !CONFIG_SMP in v4.2-rc1 on arm multi_v7_defconfig
On Fri, Jul 10 2015 at 14:29 -0600, Dave Gerlach wrote: On 07/10/2015 02:36 PM, Stephen Boyd wrote: On 07/10/2015 12:31 PM, Dave Gerlach wrote: Hello, I am seeing the following error when building v4.2-rc1 for arm with multi_v7_defconfig with CONFIG_SMP=n: LINKvmlinux LD vmlinux.o MODPOST vmlinux.o GEN .version CHK include/generated/compile.h UPD include/generated/compile.h CC init/version.o LD init/built-in.o drivers/built-in.o: In function `qcom_pm_collapse': :(.text+0xaf44c): undefined reference to `qcom_scm_cpu_power_down' drivers/built-in.o: In function `qcom_cpuidle_init': :(.init.text+0x9508): undefined reference to `qcom_scm_set_warm_boot_addr' make: *** [vmlinux] Error 1 It appears the calling functions in drivers/soc/qcom/spm.c get included by CONFIG_QCOM_PM which is part of multi_v7_defconfig but the missing functions from drivers/firmware/qcom_scm.c only get included by CONFIG_QCOM_SCM if SMP is selected. I am not sure if the correct approach is to remove CONFIG_QCOM_PM from multi_v7_defconfig or to remove 'if SMP' from CONFIG_QCOM_SCM, or something else entirely. Thoughts? CONFIG_QCOM_PM should select CONFIG_QCOM_SCM unconditionally. Ok thanks, I can send a quick patch. I sent one a few minutes ago, in reply to your mail :) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] soc: qcom: Add support for SAW2 regulators
On Fri, Dec 18 2015 at 09:15 -0700, Georgi Djakov wrote: The SAW2 (Subsystem Power Manager and Adaptive Voltage Scaling Wrapper) is part of the SPM subsystem. It is a hardware block found on some of the Qualcomm chipsets, which regulates the power to the CPU cores. Add some basic support for it, so that we can do dynamic voltage scaling. Signed-off-by: Georgi Djakov --- drivers/soc/qcom/spm.c | 149 +++- 1 file changed, 148 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c index b04b05a0904e..03fcee4b85d9 100644 --- a/drivers/soc/qcom/spm.c +++ b/drivers/soc/qcom/spm.c @@ -20,11 +20,14 @@ #include #include #include +#include #include #include #include #include #include +#include +#include #include #include @@ -51,6 +54,8 @@ enum spm_reg { SPM_REG_PMIC_DLY, SPM_REG_PMIC_DATA_0, SPM_REG_PMIC_DATA_1, + SPM_REG_RST, + SPM_REG_STS_1, SPM_REG_VCTL, SPM_REG_SEQ_ENTRY, SPM_REG_SPM_STS, @@ -68,9 +73,22 @@ struct spm_reg_data { u8 start_index[PM_SLEEP_MODE_NR]; }; +struct spm_vlevel_data { + struct spm_driver_data *drv; + unsigned selector; +}; + +struct saw2_vreg { + struct regulator_desc rdesc; + struct regulator_dev*rdev; + struct spm_driver_data *drv; + u32 selector; +}; + struct spm_driver_data { void __iomem *reg_base; const struct spm_reg_data *reg_data; + struct saw2_vreg *vreg; }; static const u8 spm_reg_offset_v2_1[SPM_REG_NR] = { @@ -94,10 +112,13 @@ static const struct spm_reg_data spm_reg_8974_8084_cpu = { static const u8 spm_reg_offset_v1_1[SPM_REG_NR] = { [SPM_REG_CFG] = 0x08, + [SPM_REG_STS_1] = 0x10, + [SPM_REG_VCTL] = 0x14, [SPM_REG_SPM_CTL] = 0x20, [SPM_REG_PMIC_DLY] = 0x24, [SPM_REG_PMIC_DATA_0] = 0x28, [SPM_REG_PMIC_DATA_1] = 0x2C, + [SPM_REG_RST] = 0x30, [SPM_REG_SEQ_ENTRY] = 0x80, }; @@ -282,6 +303,127 @@ static struct cpuidle_ops qcom_cpuidle_ops __initdata = { CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops); CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops); +static int saw2_regulator_get_voltage(struct regulator_dev *rdev) +{ + struct spm_driver_data *drv = rdev_get_drvdata(rdev); + + return regulator_list_voltage_linear_range(rdev, drv->vreg->selector); +} + +static void spm_smp_set_vdd(void *data) +{ + struct spm_vlevel_data *vdata = (struct spm_vlevel_data *)data; + struct spm_driver_data *drv = vdata->drv; + struct saw2_vreg *vreg = drv->vreg; + u32 sel = vdata->selector; + u32 val, new_val; + u32 vctl, data0, data1; + int timeout_us = 50; + + if (vreg->selector == sel) + return; + + vctl = spm_register_read(drv, SPM_REG_VCTL); + data0 = spm_register_read(drv, SPM_REG_PMIC_DATA_0); + data1 = spm_register_read(drv, SPM_REG_PMIC_DATA_1); + + /* select the band */ + val = 0x80 | sel; + + vctl &= ~0xff; + vctl |= val; + + data0 &= ~0xff; + data0 |= val; + + data1 &= ~0x3f; + data1 |= val & 0x3f; + data1 &= ~0x3f; + data1 |= (val & 0x3f) << 16; + + spm_register_write(drv, SPM_REG_RST, 1); + spm_register_write(drv, SPM_REG_VCTL, vctl); + spm_register_write(drv, SPM_REG_PMIC_DATA_0, data0); + spm_register_write(drv, SPM_REG_PMIC_DATA_1, data1); + + do { + new_val = spm_register_read(drv, SPM_REG_STS_1) & 0xff; + if (new_val == val) + break; + udelay(1); + } while (--timeout_us); + + if (!timeout_us) { + pr_err("%s: Voltage not changed: %#x\n", __func__, new_val); + return; + } + + if (sel > vreg->selector) { + /* PMIC internal slew rate is 1250 uV per us */ + udelay((sel - vreg->selector) * 10); + } + + vreg->selector = sel; +} + +static int saw2_regulator_set_voltage_sel(struct regulator_dev *rdev, + unsigned selector) +{ + struct spm_driver_data *drv = rdev_get_drvdata(rdev); + struct spm_vlevel_data data; + int cpu = rdev_get_id(rdev); + + data.drv = drv; + data.selector = selector; + + return smp_call_function_single(cpu, spm_smp_set_vdd, &data, true); +} + +static struct regulator_ops saw2_regulator_ops = { + .list_voltage = regulator_list_voltage_linear_range, + .set_voltage_sel = saw2_regulator_set_voltage_sel, + .get_voltage = saw2_regulator_get_voltage, +}; + +static struct regulator_desc saw2_regulator = { + .owner = THIS_MODULE, + .type = REGULATOR_VOLTAGE, + .ops = &saw2_regulator_ops, + .linea
Re: [PATCH] soc: qcom: rpmh: Update rpmh_invalidate function to return void
On Thu, Jun 18 2020 at 07:06 -0600, Maulik Shah wrote: Currently rpmh_invalidate() always returns success. Update its return type to void. Suggested-by: Stephen Boyd Signed-off-by: Maulik Shah Reviewed-by: Lina Iyer --- drivers/interconnect/qcom/bcm-voter.c | 6 +- drivers/soc/qcom/rpmh.c | 4 +--- include/soc/qcom/rpmh.h | 7 --- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c index 2a11a63..a3d2ef1 100644 --- a/drivers/interconnect/qcom/bcm-voter.c +++ b/drivers/interconnect/qcom/bcm-voter.c @@ -266,11 +266,7 @@ int qcom_icc_bcm_voter_commit(struct bcm_voter *voter) if (!commit_idx[0]) goto out; - ret = rpmh_invalidate(voter->dev); - if (ret) { - pr_err("Error invalidating RPMH client (%d)\n", ret); - goto out; - } + rpmh_invalidate(voter->dev); ret = rpmh_write_batch(voter->dev, RPMH_ACTIVE_ONLY_STATE, cmds, commit_idx); diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index f2b5b46c..b61e183 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -497,7 +497,7 @@ int rpmh_flush(struct rpmh_ctrlr *ctrlr) * * Invalidate the sleep and wake values in batch_cache. */ -int rpmh_invalidate(const struct device *dev) +void rpmh_invalidate(const struct device *dev) { struct rpmh_ctrlr *ctrlr = get_rpmh_ctrlr(dev); struct batch_cache_req *req, *tmp; @@ -509,7 +509,5 @@ int rpmh_invalidate(const struct device *dev) INIT_LIST_HEAD(&ctrlr->batch_cache); ctrlr->dirty = true; spin_unlock_irqrestore(&ctrlr->cache_lock, flags); - - return 0; } EXPORT_SYMBOL(rpmh_invalidate); diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h index f9ec353..bdbee1a 100644 --- a/include/soc/qcom/rpmh.h +++ b/include/soc/qcom/rpmh.h @@ -20,7 +20,7 @@ int rpmh_write_async(const struct device *dev, enum rpmh_state state, int rpmh_write_batch(const struct device *dev, enum rpmh_state state, const struct tcs_cmd *cmd, u32 *n); -int rpmh_invalidate(const struct device *dev); +void rpmh_invalidate(const struct device *dev); #else @@ -38,8 +38,9 @@ static inline int rpmh_write_batch(const struct device *dev, const struct tcs_cmd *cmd, u32 *n) { return -ENODEV; } -static inline int rpmh_invalidate(const struct device *dev) -{ return -ENODEV; } +static inline void rpmh_invalidate(const struct device *dev) +{ +} #endif /* CONFIG_QCOM_RPMH */ -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 3/3] PM / Domains: Add support for PM domain on/off notifiers for genpd
On Wed, Aug 19 2020 at 04:41 -0600, Ulf Hansson wrote: A device may have specific HW constraints that must be obeyed to, before its corresponding PM domain (genpd) can be powered off - and vice verse at power on. These constraints can't be managed through the regular runtime PM based deployment for a device, because the access pattern for it, isn't always request based. In other words, using the runtime PM callbacks to deal with the constraints doesn't work for these cases. For these reasons, let's instead add a PM domain power on/off notification mechanism to genpd. To add/remove a notifier for a device, the device must already have been attached to the genpd, which also means that it needs to be a part of the PM domain topology. To add/remove a notifier, let's introduce two genpd specific functions: - dev_pm_genpd_add|remove_notifier() Note that, to further clarify when genpd power on/off notifiers may be used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT notifiers. In the long run, the genpd power on/off notifiers should be able to replace them, but that requires additional genpd based platform support for the current users. Signed-off-by: Ulf Hansson --- drivers/base/power/domain.c | 130 ++-- include/linux/pm_domain.h | 15 + 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4b787e1ff188..9cb85a5e8342 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -545,13 +545,21 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, if (!genpd->gov) genpd->state_idx = 0; + /* Notify consumers that we are about to power off. */ + ret = raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_OFF, + NULL); + if (ret) + return ret; + /* Don't power off, if a child domain is waiting to power on. */ - if (atomic_read(&genpd->sd_count) > 0) - return -EBUSY; + if (atomic_read(&genpd->sd_count) > 0) { + ret = -EBUSY; + goto busy; + } ret = _genpd_power_off(genpd, true); if (ret) - return ret; + goto busy; genpd->status = GENPD_STATE_OFF; genpd_update_accounting(genpd); @@ -564,6 +572,9 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, } return 0; +busy: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); It would be helpful to abstract these notification related calls into functions of their own. So, for CPU PM domains, it would help to add RCU_NONIDLE() around the notifiers, allowing the callbacks to add trace functions. --Lina + return ret; } /** @@ -606,6 +617,9 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth) if (ret) goto err; + /* Inform consumers that we have powered on. */ + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); + genpd->status = GENPD_STATE_ON; genpd_update_accounting(genpd); @@ -948,9 +962,18 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock, /* Choose the deepest state when suspending */ genpd->state_idx = genpd->state_count - 1; - if (_genpd_power_off(genpd, false)) + + /* Notify consumers that we are about to power off. */ + if (raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_STATE_OFF, NULL)) return; + if (_genpd_power_off(genpd, false)) { + raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_STATE_ON, NULL); + return; + } + genpd->status = GENPD_STATE_OFF; list_for_each_entry(link, &genpd->child_links, child_node) { @@ -998,6 +1021,9 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock, _genpd_power_on(genpd, false); + /* Inform consumers that we have powered on. */ + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); + genpd->status = GENPD_STATE_ON; } @@ -1593,6 +1619,101 @@ int pm_genpd_remove_device(struct device *dev) } EXPORT_SYMBOL_GPL(pm_genpd_remove_device); +/** + * dev_pm_genpd_add_notifier - Add a genpd power on/off notifier for @dev + * + * @dev: Device that should be associated with the notifier + * @nb: The notifier block to register + * + * Users may call this function to add a genpd power on/off notifier for an + * attached @dev. Only one notifier per device is allowed. The notifier is + * sent when genpd is powering on/off the PM domain. + * + * It is assumed that the user guarantee that the genpd wouldn't be detached + * while this routine is getting called. + * + *
Re: WARNING: suspicious RCU usage - sdhci-pltfm: SDHCI platform and OF driver helper
Hi Ulf, On Tue, Sep 01 2020 at 06:41 -0600, Ulf Hansson wrote: On Tue, 1 Sep 2020 at 14:35, Ulf Hansson wrote: On Tue, 1 Sep 2020 at 12:42, wrote: > > On Tue, Sep 01, 2020 at 08:50:57AM +0200, Ulf Hansson wrote: > > On Tue, 1 Sep 2020 at 08:46, Ulf Hansson wrote: > > > On Mon, 31 Aug 2020 at 21:44, Paul E. McKenney wrote: > > > > > > [5.308588] = > > > > > [5.308593] WARNING: suspicious RCU usage > > > > > [5.316628] sdhci-pltfm: SDHCI platform and OF driver helper > > > > > [5.320052] 5.9.0-rc3 #1 Not tainted > > > > > [5.320057] - > > > > > [5.320063] /usr/src/kernel/include/trace/events/lock.h:37 suspicious rcu_dereference_check() usage! > > > > > [5.320068] > > > > > [5.320068] other info that might help us debug this: > > > > > [5.320068] > > > > > [5.320074] > > > > > [5.320074] rcu_scheduler_active = 2, debug_locks = 1 > > > > > [5.320078] RCU used illegally from extended quiescent state! > > > > > [5.320084] no locks held by swapper/0/0. > > > > > [5.320089] > > > > > [5.320089] stack backtrace: > > > > > [5.320098] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.9.0-rc3 #1 > > > > > [5.346354] sdhci_msm 7864900.sdhci: Got CD GPIO > > > > > [5.346446] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) > > > > > [5.346452] Call trace: > > > > > [5.346463] dump_backtrace+0x0/0x1f8 > > > > > [5.346471] show_stack+0x2c/0x38 > > > > > [5.346480] dump_stack+0xec/0x15c > > > > > [5.346490] lockdep_rcu_suspicious+0xd4/0xf8 > > > > > [5.346499] lock_acquire+0x3d0/0x440 > > > > > [5.346510] _raw_spin_lock_irqsave+0x80/0xb0 > > > > > [5.413118] __pm_runtime_suspend+0x34/0x1d0 > > > > > [5.417457] psci_enter_domain_idle_state+0x4c/0xb0 > > > > > [5.421795] cpuidle_enter_state+0xc8/0x610 > > > > > [5.426392] cpuidle_enter+0x3c/0x50 > > > > > [5.430561] call_cpuidle+0x44/0x80 > > > > > [5.434378] do_idle+0x240/0x2a0 > > > > > Note also that Peter Zijlstra (CCed) is working to shrink the portion > > > > of the idle loop that RCU ignores. Not sure that it covers your > > > > case, but it is worth checking. > > Right, so I think I 'caused' this by making the lock tracepoints > visible. That is, the error always existed, now we actually warn about > it. > > > > Thanks for letting me know. Let's see what Peter thinks about this then. > > > > > > Apologize for my ignorance, but from a cpuidle point of view, what > > > does it mean using RCU_NONIDLE()? I guess we should avoid RCU_NONIDLE > > > on bigger code paths? > > > > > > I could add RCU_NONIDLE for the calls to pm_runtime_put_sync_suspend() > > > and pm_runtime_get_sync() in psci_enter_domain_idle_state(). Perhaps > > > that's the easiest approach, at least to start with. > > > I think this would be nice. This should also cover the case, where PM domain power off notification callbacks call trace function internally. Right? --Lina > > > Or do you have any other ideas? > > So IMO trace_*_rcuidle() and RCU_NONIDLE() are bugs, they just mean we > got the ordering wrong and are papering over it. That said, that's been > the modus operandi for a while now, just make it shut up and don't think > about it :-/ > > That said; I pushed the rcu_idle_enter() about as deep as it goes into > generic code in commit 1098582a0f6c ("sched,idle,rcu: Push rcu_idle > deeper into the idle path") Aha, that commit should fix this problem, I think. Looks like that commit was sent as a fix and included in the recent v5.9-rc3. Naresh, can you try with the above commit? Ah, just realized that I misread the patch. It doesn't fix it. We would still need a RCU_NONIDLE() in psci_enter_domain_idle_state() - or something along the lines of what you suggest below. Apologies for the noise. Kind regards Uffe [1]. https://lkml.org/lkml/2020/8/26/81 > > I suppose the next step is pushing it into individual driver when > needed, something like the below perhaps. I realize the coupled idle > state stuff is more complicated that most, but it's also not an area > I've looked at in detail, so perhaps I've just made a bigger mess, but > it ought to give you enough to get going I think. These aren't coupled states. Instead, in cpuidle-psci, we are using PM domains through genpd and runtime PM to manage "shared idle states" between CPUs. Kind regards Uffe > > Rafael? > > --- > diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c > index 74463841805f..617bbef316e6 100644 > --- a/drivers/cpuidle/cpuidle-psci.c > +++ b/drivers/cpuidle/cpuidle-psci.c > @@ -49,6 +49,9 @@ static inline u32 psci_get_domain_state(void) > > static inline int psci_enter_state(int idx, u32 state) > { > + /* > +* XXX push rcu_idle_enter into the coupled code > +*/ > return CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, idx, state); > } > > @@ -72,7 +75
Re: [PATCH v3] PM: domains: Add support for PM domain on/off notifiers for genpd
On Tue, Oct 13 2020 at 06:23 -0600, Ulf Hansson wrote: A device may have specific HW constraints that must be obeyed to, before its corresponding PM domain (genpd) can be powered off - and vice verse at power on. These constraints can't be managed through the regular runtime PM based deployment for a device, because the access pattern for it, isn't always request based. In other words, using the runtime PM callbacks to deal with the constraints doesn't work for these cases. For these reasons, let's instead add a PM domain power on/off notification mechanism to genpd. To add/remove a notifier for a device, the device must already have been attached to the genpd, which also means that it needs to be a part of the PM domain topology. To add/remove a notifier, let's introduce two genpd specific functions: - dev_pm_genpd_add|remove_notifier() Note that, to further clarify when genpd power on/off notifiers may be used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT notifiers. In the long run, the genpd power on/off notifiers should be able to replace them, but that requires additional genpd based platform support for the current users. Signed-off-by: Ulf Hansson --- Changes in v3: - Adopted suggestions from Peng Fan to allow more fine grained levels of notifications, which is needed on some NXP platforms. - Move the code that fires the notifications into _genpd_power_on|off(), as it simply fits better in there. Note that, I understand that some of us may be occupied with dealing with the merge window, but I still wanted to get this submitted to allow those that have some time to review and test. Kind regards Ulf Hansson --- drivers/base/power/domain.c | 161 +--- include/linux/pm_domain.h | 22 + 2 files changed, 171 insertions(+), 12 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 05bb4d4401b2..c2a8821bdb26 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -413,28 +413,47 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; - int ret; + int ret, nr_calls = 0; + + /* Notify consumers that we are about to power on. */ + ret = __raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_NOTIFY_PRE_ON, NULL, -1, + &nr_calls); Rafael's bleeding-edge fails to compile because this call is removed now - https://lore.kernel.org/lkml/20200818135804.325626...@infradead.org/ which also handles the failure case (_robust) variant. --Lina + ret = notifier_to_errno(ret); + if (ret) + goto err; if (!genpd->power_on) - return 0; + goto out; - if (!timed) - return genpd->power_on(genpd); + if (!timed) { + ret = genpd->power_on(genpd); + if (ret) + goto err; + + goto out; + } time_start = ktime_get(); ret = genpd->power_on(genpd); if (ret) - return ret; + goto err; elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); if (elapsed_ns <= genpd->states[state_idx].power_on_latency_ns) - return ret; + goto out; genpd->states[state_idx].power_on_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "on", elapsed_ns); +out: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL); + return 0; +err: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF, + NULL); return ret; } @@ -443,29 +462,51 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; - int ret; + int ret, nr_calls = 0; + + /* Notify consumers that we are about to power off. */ + ret = __raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_NOTIFY_PRE_OFF, NULL, -1, + &nr_calls); + ret = notifier_to_errno(ret); + if (ret) + goto busy; if (!genpd->power_off) - return 0; + goto out; + + if (!timed) { + ret = genpd->power_off(genpd); + if (ret) + goto busy; - if (!timed) - return genpd->power_off(genpd); + goto out; + } time_start = ktime_get(); ret = genpd->power_off(genpd); if (ret) - return ret; +
Re: [PATCH] soc: qcom: QCOM_RPMH fix build with modular QCOM_RPMH
Hi Anders, On Tue, Oct 27 2020 at 05:14 -0600, Anders Roxell wrote: When building allmodconfig leading to the following link error with CONFIG_QCOM_RPMH=y and CONFIG_QCOM_COMMAND_DB=m: aarch64-linux-gnu-ld: drivers/clk/qcom/clk-rpmh.o: in function `clk_rpmh_probe': drivers/clk/qcom/clk-rpmh.c:474: undefined reference to `cmd_db_read_addr' drivers/clk/qcom/clk-rpmh.c:474:(.text+0x254): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `cmd_db_read_addr' Fix this by adding a Kconfig depenency and forcing QCOM_RPMH to be a module when QCOM_COMMAND_DB is a module. Also removing the dependency on 'ARCH_QCOM || COMPILE_TEST' since that is already a dependency for QCOM_COMMAND_DB. Fixes: 778279f4f5e4 ("soc: qcom: cmd-db: allow loading as a module") Signed-off-by: Anders Roxell --- drivers/soc/qcom/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index 9b4ae9c16ba7..3bdd1604f78f 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -109,7 +109,7 @@ config QCOM_RMTFS_MEM config QCOM_RPMH tristate "Qualcomm RPM-Hardened (RPMH) Communication" - depends on ARCH_QCOM || COMPILE_TEST + depends on QCOM_COMMAND_DB A solution was posted in the mailing list alredy - https://lore.kernel.org/linux-arm-msm/20201008040907.7036-1-il...@codeaurora.org/ If you get a chance, please give that a shot to see if that works for you. Thanks, Lina help Support for communication with the hardened-RPM blocks in Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an -- 2.28.0
Re: [PATCH] soc: qcom: QCOM_RPMH fix build with modular QCOM_RPMH
On Wed, Oct 28 2020 at 03:43 -0600, Anders Roxell wrote: On Tue, 27 Oct 2020 at 22:15, Lina Iyer wrote: Hi Anders, On Tue, Oct 27 2020 at 05:14 -0600, Anders Roxell wrote: >When building allmodconfig leading to the following link error with >CONFIG_QCOM_RPMH=y and CONFIG_QCOM_COMMAND_DB=m: > >aarch64-linux-gnu-ld: drivers/clk/qcom/clk-rpmh.o: in function `clk_rpmh_probe': > drivers/clk/qcom/clk-rpmh.c:474: undefined reference to `cmd_db_read_addr' > drivers/clk/qcom/clk-rpmh.c:474:(.text+0x254): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `cmd_db_read_addr' > >Fix this by adding a Kconfig depenency and forcing QCOM_RPMH to be a >module when QCOM_COMMAND_DB is a module. Also removing the dependency on >'ARCH_QCOM || COMPILE_TEST' since that is already a dependency for >QCOM_COMMAND_DB. > >Fixes: 778279f4f5e4 ("soc: qcom: cmd-db: allow loading as a module") >Signed-off-by: Anders Roxell >--- > drivers/soc/qcom/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >index 9b4ae9c16ba7..3bdd1604f78f 100644 >--- a/drivers/soc/qcom/Kconfig >+++ b/drivers/soc/qcom/Kconfig >@@ -109,7 +109,7 @@ config QCOM_RMTFS_MEM > > config QCOM_RPMH > tristate "Qualcomm RPM-Hardened (RPMH) Communication" >- depends on ARCH_QCOM || COMPILE_TEST >+ depends on QCOM_COMMAND_DB A solution was posted in the mailing list alredy - https://lore.kernel.org/linux-arm-msm/20201008040907.7036-1-il...@codeaurora.org/ I missed that one, thanks. If you get a chance, please give that a shot to see if that works for you. That will work too, but the "depends on ARCH_QCOM || COMPILE_TEST" isn't needed since that is already the dependency for QCOM_COMMAND_DB. So that should be met here too or am I missing something? Sure, if you want to post an update to the patch, that would be fine too. Bjorn: Have you picked up this patch yet? If he hasn't please feel free to update the patch. Or, I can do that as well. --Lina Cheers, Anders Thanks, Lina > help > Support for communication with the hardened-RPM blocks in > Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an >-- >2.28.0 >
Re: [PATCH v3] PM: domains: Add support for PM domain on/off notifiers for genpd
On Tue, Oct 13 2020 at 06:23 -0600, Ulf Hansson wrote: A device may have specific HW constraints that must be obeyed to, before its corresponding PM domain (genpd) can be powered off - and vice verse at power on. These constraints can't be managed through the regular runtime PM based deployment for a device, because the access pattern for it, isn't always request based. In other words, using the runtime PM callbacks to deal with the constraints doesn't work for these cases. For these reasons, let's instead add a PM domain power on/off notification mechanism to genpd. To add/remove a notifier for a device, the device must already have been attached to the genpd, which also means that it needs to be a part of the PM domain topology. To add/remove a notifier, let's introduce two genpd specific functions: - dev_pm_genpd_add|remove_notifier() Note that, to further clarify when genpd power on/off notifiers may be used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT notifiers. In the long run, the genpd power on/off notifiers should be able to replace them, but that requires additional genpd based platform support for the current users. Signed-off-by: Ulf Hansson --- Changes in v3: - Adopted suggestions from Peng Fan to allow more fine grained levels of notifications, which is needed on some NXP platforms. - Move the code that fires the notifications into _genpd_power_on|off(), as it simply fits better in there. Note that, I understand that some of us may be occupied with dealing with the merge window, but I still wanted to get this submitted to allow those that have some time to review and test. Kind regards Ulf Hansson --- drivers/base/power/domain.c | 161 +--- include/linux/pm_domain.h | 22 + 2 files changed, 171 insertions(+), 12 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 05bb4d4401b2..c2a8821bdb26 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -413,28 +413,47 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; - int ret; + int ret, nr_calls = 0; + + /* Notify consumers that we are about to power on. */ + ret = __raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_NOTIFY_PRE_ON, NULL, -1, + &nr_calls); + ret = notifier_to_errno(ret); + if (ret) + goto err; if (!genpd->power_on) - return 0; + goto out; - if (!timed) - return genpd->power_on(genpd); + if (!timed) { + ret = genpd->power_on(genpd); + if (ret) + goto err; + + goto out; + } time_start = ktime_get(); ret = genpd->power_on(genpd); if (ret) - return ret; + goto err; elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); if (elapsed_ns <= genpd->states[state_idx].power_on_latency_ns) - return ret; + goto out; genpd->states[state_idx].power_on_latency_ns = elapsed_ns; genpd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "on", elapsed_ns); +out: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL); + return 0; +err: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF, + NULL); return ret; } @@ -443,29 +462,51 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) unsigned int state_idx = genpd->state_idx; ktime_t time_start; s64 elapsed_ns; - int ret; + int ret, nr_calls = 0; + + /* Notify consumers that we are about to power off. */ + ret = __raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_NOTIFY_PRE_OFF, NULL, -1, + &nr_calls); + ret = notifier_to_errno(ret); + if (ret) + goto busy; Nit: You could enclose this in a function call to keep these functions clean. Otherwise, Tested-by: Lina Iyer if (!genpd->power_off) - return 0; + goto out; + + if (!timed) { + ret = genpd->power_off(genpd); + if (ret) + goto busy; - if (!timed) - return genpd->power_off(genpd); + goto out; + } time_start = ktime_get(); ret = genpd->power_off(genpd); if (r
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
On Mon, Jul 27 2020 at 18:45 -0600, Stephen Boyd wrote: Quoting Lina Iyer (2020-07-24 09:28:25) On Fri, Jul 24 2020 at 03:03 -0600, Rajendra Nayak wrote: >Hi Maulik/Lina, > >On 7/23/2020 11:36 PM, Stanimir Varbanov wrote: >>Hi Rajendra, >> >>After applying 2,3 and 4/5 patches on linaro-integration v5.8-rc2 I see >>below messages on db845: >> >>qcom-venus aa0.video-codec: dev_pm_opp_set_rate: failed to find >>current OPP for freq 53397 (-34) >> >>^^^ This one is new. >> >>qcom_rpmh TCS Busy, retrying RPMH message send: addr=0x3 >> >>^^^ and this message is annoying, can we make it pr_debug in rpmh? > How annoyingly often do you see this message? Usually, this is an indication of bad system state either on remote processors in the SoC or in Linux itself. On a smooth sailing build you should not see this 'warning'. >Would you be fine with moving this message to a pr_debug? Its currently >a pr_info_ratelimited() I would rather not, moving this out of sight will mask a lot serious issues that otherwise bring attention to the developers. I removed this warning message in my patch posted to the list[1]. If it's a serious problem then I suppose a timeout is more appropriate, on the order of several seconds or so and then a pr_warn() and bail out of the async call with an error. The warning used to capture issues that happen within a second and it helps capture system related issues. Timing out after many seconds overlooks the system issues that generally tend to resolve itself, but nevertheless need to be investigated. --Lina [1] https://lore.kernel.org/r/20200724211711.810009-1-sb...@kernel.org
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
On Tue, Jul 28 2020 at 13:51 -0600, Stephen Boyd wrote: Quoting Lina Iyer (2020-07-28 09:52:12) On Mon, Jul 27 2020 at 18:45 -0600, Stephen Boyd wrote: >Quoting Lina Iyer (2020-07-24 09:28:25) >> On Fri, Jul 24 2020 at 03:03 -0600, Rajendra Nayak wrote: >> >Hi Maulik/Lina, >> > >> >On 7/23/2020 11:36 PM, Stanimir Varbanov wrote: >> >>Hi Rajendra, >> >> >> >>After applying 2,3 and 4/5 patches on linaro-integration v5.8-rc2 I see >> >>below messages on db845: >> >> >> >>qcom-venus aa0.video-codec: dev_pm_opp_set_rate: failed to find >> >>current OPP for freq 53397 (-34) >> >> >> >>^^^ This one is new. >> >> >> >>qcom_rpmh TCS Busy, retrying RPMH message send: addr=0x3 >> >> >> >>^^^ and this message is annoying, can we make it pr_debug in rpmh? >> > >> How annoyingly often do you see this message? >> Usually, this is an indication of bad system state either on remote >> processors in the SoC or in Linux itself. On a smooth sailing build you >> should not see this 'warning'. >> >> >Would you be fine with moving this message to a pr_debug? Its currently >> >a pr_info_ratelimited() >> I would rather not, moving this out of sight will mask a lot serious >> issues that otherwise bring attention to the developers. >> > >I removed this warning message in my patch posted to the list[1]. If >it's a serious problem then I suppose a timeout is more appropriate, on >the order of several seconds or so and then a pr_warn() and bail out of >the async call with an error. > The warning used to capture issues that happen within a second and it helps capture system related issues. Timing out after many seconds overlooks the system issues that generally tend to resolve itself, but nevertheless need to be investigated. Is it correct to read "system related issues" as performance problems where the thread is spinning forever trying to send a message and it can't? So the problem is mostly that it's an unbounded amount of time before the message is sent to rpmh and this printk helps identify those situations where that is happening? Yes, but mostly a short period of time like when other processors are in the middle of a restart or resource states changes have taken unusual amounts of time. The system will generally recover from this without crashing in this case. User action is investigation of the situation leading to these messages. Otherwise as you say above it's a bad system state where the rpmh processor has gotten into a bad state like a crash? Can we recover from that? Or is the only recovery a reboot of the system? Does the rpmh processor reboot the system if it crashes? We cannot recover from such a state. The remote processor will reboot if it detects a failure at it's end. If the system entered a bad state, it is possible that RPMH requests start timing out in Linux and remote processor may not detect it. Hence, the timeout in rpmh_write() API. The advised course of action is a restart as there is no way to recover from this state. --Lina
Re: [PATCH] soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free
On Wed, Jul 22 2020 at 19:01 -0600, Stephen Boyd wrote: The busy loop in rpmh_rsc_send_data() is written with the assumption that the udelay will be preempted by the tcs_tx_done() irq handler when the TCS slots are all full. This doesn't hold true when the calling thread is an irqthread and the tcs_tx_done() irq is also an irqthread. That's because kernel irqthreads are SCHED_FIFO and thus need to voluntarily give up priority by calling into the scheduler so that other threads can run. I see RCU stalls when I boot with irqthreads on the kernel commandline because the modem remoteproc driver is trying to send an rpmh async message from an irqthread that needs to give up the CPU for the rpmh irqthread to run and clear out tcs slots. Would this be not better, if we we use a threaded IRQ handler or offload tx_done to another waitqueue instead of handling it in IRQ handler? -- Lina rcu: INFO: rcu_preempt self-detected stall on CPU rcu: 0-: (1 GPs behind) idle=402/1/0x4002 softirq=2108/2109 fqs=4920 (t=21016 jiffies g=2933 q=590) Task dump for CPU 0: irq/11-smp2pR running task0 148 2 0x0028 Call trace: dump_backtrace+0x0/0x154 show_stack+0x20/0x2c sched_show_task+0xfc/0x108 dump_cpu_task+0x44/0x50 rcu_dump_cpu_stacks+0xa4/0xf8 rcu_sched_clock_irq+0x7dc/0xaa8 update_process_times+0x30/0x54 tick_sched_handle+0x50/0x64 tick_sched_timer+0x4c/0x8c __hrtimer_run_queues+0x21c/0x36c hrtimer_interrupt+0xf0/0x22c arch_timer_handler_phys+0x40/0x50 handle_percpu_devid_irq+0x114/0x25c __handle_domain_irq+0x84/0xc4 gic_handle_irq+0xd0/0x178 el1_irq+0xbc/0x180 save_return_addr+0x18/0x28 return_address+0x54/0x88 preempt_count_sub+0x40/0x88 _raw_spin_unlock_irqrestore+0x4c/0x6c ___ratelimit+0xd0/0x128 rpmh_rsc_send_data+0x24c/0x378 __rpmh_write+0x1b0/0x208 rpmh_write_async+0x90/0xbc rpmhpd_send_corner+0x60/0x8c rpmhpd_aggregate_corner+0x8c/0x124 rpmhpd_set_performance_state+0x8c/0xbc _genpd_set_performance_state+0xdc/0x1b8 dev_pm_genpd_set_performance_state+0xb8/0xf8 q6v5_pds_disable+0x34/0x60 [qcom_q6v5_mss] qcom_msa_handover+0x38/0x44 [qcom_q6v5_mss] q6v5_handover_interrupt+0x24/0x3c [qcom_q6v5] handle_nested_irq+0xd0/0x138 qcom_smp2p_intr+0x188/0x200 irq_thread_fn+0x2c/0x70 irq_thread+0xfc/0x14c kthread+0x11c/0x12c ret_from_fork+0x10/0x18 This busy loop naturally lends itself to using a wait queue so that each thread that tries to send a message will sleep waiting on the waitqueue and only be woken up when a free slot is available. This should make things more predictable too because the scheduler will be able to sleep tasks that are waiting on a free tcs instead of the busy loop we currently have today. Cc: Douglas Anderson Cc: Maulik Shah Cc: Lina Iyer Signed-off-by: Stephen Boyd --- drivers/soc/qcom/rpmh-internal.h | 2 + drivers/soc/qcom/rpmh-rsc.c | 101 --- 2 files changed, 41 insertions(+), 62 deletions(-) diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index ef60e790a750..9a325bac58fe 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -8,6 +8,7 @@ #define __RPM_INTERNAL_H__ #include +#include #include #define TCS_TYPE_NR 4 @@ -118,6 +119,7 @@ struct rsc_drv { struct tcs_group tcs[TCS_TYPE_NR]; DECLARE_BITMAP(tcs_in_use, MAX_TCS_NR); spinlock_t lock; + wait_queue_head_t tcs_wait; struct rpmh_ctrlr client; }; diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 076fd27f3081..6c758b052c95 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -444,6 +445,7 @@ static irqreturn_t tcs_tx_done(int irq, void *p) */ if (!drv->tcs[ACTIVE_TCS].num_tcs) enable_tcs_irq(drv, i, false); + wake_up(&drv->tcs_wait); spin_unlock(&drv->lock); if (req) rpmh_tx_done(req, err); @@ -562,44 +564,59 @@ static int find_free_tcs(struct tcs_group *tcs) return -EBUSY; } +static int claim_tcs_for_req(struct rsc_drv *drv, struct tcs_group *tcs, +const struct tcs_request *msg) +{ + int ret; + + /* +* The h/w does not like if we send a request to the same address, +* when one is already in-flight or being processed. +*/ + ret = check_for_req_inflight(drv, tcs, msg); + if (ret) + return ret; + + return find_free_tcs(tcs); +} + /** - * tcs_write() - Store messages into a TCS right now, or return -EBUSY. + * rpmh_rsc_send_data() - Write / trigger active-only message. * @drv: The controller. * @msg: The data to be sent. * - * Grabs a TCS for ACTIVE_ONLY transfers and writes the messages to it. - * - * If
Re: [PATCH v4 4/5] arm64: dts: sdm845: Add OPP tables and power-domains for venus
On Fri, Jul 24 2020 at 03:03 -0600, Rajendra Nayak wrote: Hi Maulik/Lina, On 7/23/2020 11:36 PM, Stanimir Varbanov wrote: Hi Rajendra, After applying 2,3 and 4/5 patches on linaro-integration v5.8-rc2 I see below messages on db845: qcom-venus aa0.video-codec: dev_pm_opp_set_rate: failed to find current OPP for freq 53397 (-34) ^^^ This one is new. qcom_rpmh TCS Busy, retrying RPMH message send: addr=0x3 ^^^ and this message is annoying, can we make it pr_debug in rpmh? How annoyingly often do you see this message? Usually, this is an indication of bad system state either on remote processors in the SoC or in Linux itself. On a smooth sailing build you should not see this 'warning'. Would you be fine with moving this message to a pr_debug? Its currently a pr_info_ratelimited() I would rather not, moving this out of sight will mask a lot serious issues that otherwise bring attention to the developers. --Lina
Re: [PATCH] soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free
On Fri, Jul 24 2020 at 14:01 -0600, Stephen Boyd wrote: Quoting Doug Anderson (2020-07-24 12:49:56) Hi, On Fri, Jul 24, 2020 at 12:44 PM Stephen Boyd wrote: I think Lina was alluding to this earlier in this thread. I was thinking more of threaded irq handler than a kthread to post the requests. We went away from post-thread approach a few years ago.
Re: [PATCH] soc: qcom: rpmh-rsc: Sleep waiting for tcs slots to be free
On Fri, Jul 24 2020 at 14:11 -0600, Stephen Boyd wrote: Quoting Lina Iyer (2020-07-24 13:08:41) On Fri, Jul 24 2020 at 14:01 -0600, Stephen Boyd wrote: >Quoting Doug Anderson (2020-07-24 12:49:56) >> Hi, >> >> On Fri, Jul 24, 2020 at 12:44 PM Stephen Boyd wrote: >I think Lina was alluding to this earlier in this >thread. I was thinking more of threaded irq handler than a kthread to post the requests. We went away from post-thread approach a few years ago. Ok, got it. Why was the kthread approach abandoned? Simplification mostly, I think. Also, somebody requested that when the async call returns they would atleast be guaranteed that the request has been posted not necessarily processed at the remote end.
Re: [PATCH 3/3] PM / Domains: Add support for PM domain on/off notifiers for genpd
-Original Message- Date: Wed, 19 Aug 2020 12:40:57 +0200 From: Ulf Hansson To: "Rafael J . Wysocki" , Kevin Hilman , linux...@vger.kernel.org Cc: Sudeep Holla , Lorenzo Pieralisi , Daniel Lezcano , Lina Iyer , Lukasz Luba , Vincent Guittot , Stephen Boyd , Bjorn Andersson , Benjamin Gaignard , Ulf Hansson , linux-arm-ker...@lists.infradead.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/3] PM / Domains: Add support for PM domain on/off notifiers for genpd A device may have specific HW constraints that must be obeyed to, before its corresponding PM domain (genpd) can be powered off - and vice verse at power on. These constraints can't be managed through the regular runtime PM based deployment for a device, because the access pattern for it, isn't always request based. In other words, using the runtime PM callbacks to deal with the constraints doesn't work for these cases. For these reasons, let's instead add a PM domain power on/off notification mechanism to genpd. To add/remove a notifier for a device, the device must already have been attached to the genpd, which also means that it needs to be a part of the PM domain topology. To add/remove a notifier, let's introduce two genpd specific functions: - dev_pm_genpd_add|remove_notifier() Note that, to further clarify when genpd power on/off notifiers may be used, one can compare with the existing CPU_CLUSTER_PM_ENTER|EXIT notifiers. In the long run, the genpd power on/off notifiers should be able to replace them, but that requires additional genpd based platform support for the current users. Signed-off-by: Ulf Hansson --- drivers/base/power/domain.c | 130 ++-- include/linux/pm_domain.h | 15 + 2 files changed, 141 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4b787e1ff188..9cb85a5e8342 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -545,13 +545,21 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, if (!genpd->gov) genpd->state_idx = 0; + /* Notify consumers that we are about to power off. */ + ret = raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_OFF, + NULL); + if (ret) + return ret; + /* Don't power off, if a child domain is waiting to power on. */ - if (atomic_read(&genpd->sd_count) > 0) - return -EBUSY; + if (atomic_read(&genpd->sd_count) > 0) { + ret = -EBUSY; + goto busy; + } ret = _genpd_power_off(genpd, true); if (ret) - return ret; + goto busy; genpd->status = GENPD_STATE_OFF; genpd_update_accounting(genpd); @@ -564,6 +572,9 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, } return 0; +busy: + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); + return ret; } /** @@ -606,6 +617,9 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth) if (ret) goto err; + /* Inform consumers that we have powered on. */ + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); + genpd->status = GENPD_STATE_ON; genpd_update_accounting(genpd); @@ -948,9 +962,18 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock, /* Choose the deepest state when suspending */ genpd->state_idx = genpd->state_count - 1; - if (_genpd_power_off(genpd, false)) + + /* Notify consumers that we are about to power off. */ + if (raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_STATE_OFF, NULL)) return; If any of the notified returns an error, we probably should inform that the genpd failed as well. -- Lina + if (_genpd_power_off(genpd, false)) { + raw_notifier_call_chain(&genpd->power_notifiers, + GENPD_STATE_ON, NULL); + return; + } + genpd->status = GENPD_STATE_OFF; list_for_each_entry(link, &genpd->child_links, child_node) { @@ -998,6 +1021,9 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock, _genpd_power_on(genpd, false); + /* Inform consumers that we have powered on. */ + raw_notifier_call_chain(&genpd->power_notifiers, GENPD_STATE_ON, NULL); + genpd->status = GENPD_STATE_ON; } @@ -1593,6 +1619,101 @@ int pm_genpd_remove_device(struct device *dev) } EXPORT_SYMBOL_GPL(pm_genpd_remove_device); +/** + * dev_pm_genpd_add_notifier - Add a genpd power on/off notifie
Re: [PATCH v4 00/10] support wakeup capable GPIOs
On Mon, Apr 15 2019 at 06:43 -0600, Marc Zyngier wrote: On 13/03/2019 21:18, Lina Iyer wrote: Hi all, This series adds support for wakeup capable GPIOs. It is based on Thierry's hiearchical GPIO domains. This approach is based on Stephen's idea [1]. The SoC that is used for this development is a QCOM SDM845. The current patchset is rebased on top of 5.0 and adds documentation for the wakeup-parent and irqdomain-map DT properties along with the the optional irqdomain-map-mask and irqdomain-map-pass-thru properties. Also incorporating comments from Marc on the earlier submission [2]. I cleaned up some of the change history in these patches to match the version number with that of the submission. The dtsi patches are based on Bjorn's changes for increased address and cell size [3] and [4]. Kindly review the series. What the status of this? What is the expected merge strategy? Hi Mark, I ran into a couple of issues, most of which have been sorted out. But there is a hardware requirement to write up another register (to set type), for GPIO wakeup that was missed earlier. I am trying to get that tested out. That's the hold up. Sorry about that. --Lina
Re: [PATCH v2 0/8] qcom: support wakeup capable GPIOs
On Mon, Jan 28 2019 at 07:19 -0700, Linus Walleij wrote: On Thu, Jan 24, 2019 at 9:22 PM Lina Iyer wrote: This is a bug fix submission of the v1 posted here [1]. The discussion on how to represent the wakeup-parent interrupt controller is on-going [2] here. The reiew comments in [1], from Doug and Stephen are addressed in this patch. The series attempts to add GPIO chip in hierarchy with PDC interrupt controller that can detect and wakeup the GPIOs routed to it, when the system is suspend/deep sleep mode. I kind of start to get the hang of this now. This is starting to look finished. Some words on the hierarchical GPIO IRQs: I have started to look into hierarchical GPIO+irqchip since the usage of such is spreading. I have been on to Thierry patches trying to make him implement more generic helpers in the gpiolib irqchip library functions. In short I see the following: - Hierarchical gpiochips all have .alloc() and .translate() functions that look more or less the same. Yes. - They all fall down to remapping either tuples or entire ranges of IRQs from parent to child with a linear look-up table on allocation. I would think this would be the generic case, unless its QCOM, where we would want to push the tuples up hierarchy :( So my idea would be to add support for this into the gpiolib hierarchical irqchip helper: by supplying a parent irqdomain and a list of translation tuples, we should be able to handle pretty much any hierarchical GPIO irqdomain (famous last words). We would read the tuples from DT? Also please consider Rob's idea of specifying hierarchy from [2]. Right now I am converting the IXP4xx platform to hierarchical IRQ from the ground up (it is not even using device tree so it is a bit of a challenge) but it seems to be working. So I will try to post this in some hopefully working form, and on top of those changes or before them introduce some helpers in the core for hierarchical irqs. Or I fail. Thanks, please copy me on the thread. I would not want to miss this. Anyways, I do not think my ambitions for refactoring the helpers can stand in the way of support for these use cases and new hardware, so maybe we need to merge a few more hierarchical drivers just bypassing the gpiolib helpers. I don't want to block development, and I suspect Thierry might be getting annoyed at me at this point, so we should maybe just pile up a few more hierarchical chips so I can refactor them later. What do you think? I attempted refactoring the .alloc and .translate and for a generic case and it seemed like it would fit the bill very well. Will await your patches. Thanks, Lina
Re: [PATCH v2 5/8] drivers: pinctrl: msm: setup GPIO irqchip hierarchy
On Wed, Jan 30 2019 at 15:45 -0700, Stephen Boyd wrote: Quoting Lina Iyer (2019-01-24 12:22:02) To allow GPIOs to wakeup the system from suspend or deep idle, the wakeup capable GPIOs are setup in hierarchy with interrupts from the wakeup-parent irqchip. In older SoC's, the TLMM will handover detection to the parent irqchip and in newer SoC's, the parent irqchip may also be active as well as the TLMM and therefore the GPIOs need to be masked at TLMM to avoid duplicate interrupts. To enable both these configurations to exist, allow the parent irqchip to dictate the TLMM irqchip's behavior when masking/unmasking the interrupt. Co-developed-by: Stephen Boyd Signed-off-by: Lina Iyer --- Changes in v2: - Fix bug when unmaksing PDC interrupt What was the bug? The problem was an incorrect merge (possibly manual), causing the PDC to be not used at all. This is what I had - static void msm_gpio_irq_mask(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct msm_pinctrl *pctrl = gpiochip_get_data(gc); const struct msm_pingroup *g; unsigned long flags; u32 val; g = &pctrl->soc->groups[d->hwirq]; if (d->parent_data) irq_chip_unmask_parent(d); /* Monitored by parent wakeup controller? Keep masked */ if (test_bit(d->hwirq, pctrl->wakeup_masked_irqs)) return; The above unmask_parent() call in an irq_mask() callback was the bug. Is that why the mask callback in this gpio chip no longer calls the parent irq chip? We should keep calling the parent irqchip from what I can tell. Otherwise, we may never mask the irq at the PDC and only mask it at the GPIO level, which may not even care about it if it's being monitored by the PDC. This causes me to get a bunch of interrupts on my touchscreen when I touch it once vs. only a handful (like 4) when I fix it with the below patch: Hmm... I did not see an issue in my local testing :( Thanks for the fix. Can you fold it in? Yes, I will fold it in and send a rev out with the fix. Thanks, Lina diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index dd72ec8fb8db..9b45219893bd 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -682,6 +682,9 @@ static void msm_gpio_irq_mask(struct irq_data *d) clear_bit(d->hwirq, pctrl->enabled_irqs); raw_spin_unlock_irqrestore(&pctrl->lock, flags); + + if (d->parent_data) + irq_chip_mask_parent(d); } static void msm_gpio_irq_unmask(struct irq_data *d)
Re: [RFC][PATCH 3/5] irqchip: Allow QCOM_PDC to be loadable as a perment module
On Tue, Jun 16 2020 at 05:30 -0600, Maulik Shah wrote: Hi, On 6/16/2020 11:43 AM, John Stultz wrote: Allows qcom-pdc driver to be loaded as a permenent module typo: permanent Also, due to the fact that IRQCHIP_DECLARE becomes a no-op when building as a module, we have to add the platform driver hooks explicitly. Thanks to Saravana for his help on pointing out the IRQCHIP_DECLARE issue and guidance on a solution. Cc: Andy Gross Cc: Bjorn Andersson Cc: Joerg Roedel Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier Cc: Linus Walleij Cc: Lina Iyer Cc: Saravana Kannan Cc: Todd Kjos Cc: Greg Kroah-Hartman Cc: linux-arm-...@vger.kernel.org Cc: io...@lists.linux-foundation.org Cc: linux-g...@vger.kernel.org Signed-off-by: John Stultz --- drivers/irqchip/Kconfig| 2 +- drivers/irqchip/qcom-pdc.c | 30 ++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 29fead208cad..12765bed08f9 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -425,7 +425,7 @@ config GOLDFISH_PIC for Goldfish based virtual platforms. config QCOM_PDC - bool "QCOM PDC" + tristate "QCOM PDC" depends on ARCH_QCOM select IRQ_DOMAIN_HIERARCHY help diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index 6ae9e1f0819d..98d74160afcd 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -11,7 +11,9 @@ #include #include #include +#include #include +#include please move this include in order after of_device.h #include #include #include @@ -430,4 +432,32 @@ static int qcom_pdc_init(struct device_node *node, struct device_node *parent) return ret; } +#ifdef MODULE +static int qcom_pdc_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct device_node *parent = of_irq_find_parent(np); + + return qcom_pdc_init(np, parent); +} + +static const struct of_device_id qcom_pdc_match_table[] = { + { .compatible = "qcom,pdc" }, + {} +}; +MODULE_DEVICE_TABLE(of, qcom_pdc_match_table); + +static struct platform_driver qcom_pdc_driver = { + .probe = qcom_pdc_probe, + .driver = { + .name = "qcom-pdc", + .of_match_table = qcom_pdc_match_table, can you please set .suppress_bind_attrs = true, This is to prevent bind/unbind using sysfs. Once irqchip driver module is loaded, it shouldn't get unbind at runtime. That is a good point. We probably should do that to RPMH RSC driver as well. Thanks, Maulik + }, +}; +module_platform_driver(qcom_pdc_driver); +#else IRQCHIP_DECLARE(qcom_pdc, "qcom,pdc", qcom_pdc_init); +#endif + +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Power Domain Controller"); +MODULE_LICENSE("GPL v2"); -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v3 2/2] dt-bindings: introduce Command DB for QCOM SoCs
On Mon, Feb 19 2018 at 20:31 +, Rob Herring wrote: On Fri, Feb 16, 2018 at 03:46:14PM -0700, Lina Iyer wrote: From: Mahesh Sivasubramanian +The devicetree representation of the command DB driver should be: Need to state this is a child of /reserved-memory Yes, ofcourse. Will fix. + +PROPERTIES: Why the yelling? :) Oops. +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,cmd-db" reg? Yes. Thanks Rob for the reveiw. -- Lina
Re: [PATCH v2 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS
Hi Evan, Thanks for your review. On Fri, Feb 16 2018 at 23:51 +, Evan Green wrote: Hello Lina, On Thu, Feb 15, 2018 at 9:35 AM, Lina Iyer wrote: Sleep and wake requests are sent when the application processor subsystem of the SoC is entering deep sleep states like in suspend. These requests help lower the system power requirements when the resources are not in use. Sleep and wake requests are written to the TCS slots but are not triggered at the time of writing. The TCS are triggered by the firmware after the last of the CPUs has executed its WFI. Since these requests may come in different batches of requests, it is job of this controller driver to find arrange the requests into the available TCSes. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh-internal.h | 7 +++ drivers/soc/qcom/rpmh-rsc.c | 126 +++ 2 files changed, 133 insertions(+) [...] +static int find_slots(struct tcs_group *tcs, struct tcs_request *msg, +int *m, int *n) +{ + int slot, offset; + int i = 0; + + /* Find if we already have the msg in our TCS */ + slot = find_match(tcs, msg->payload, msg->num_payload); + if (slot >= 0) + goto copy_data; + + /* Do over, until we can fit the full payload in a TCS */ + do { + slot = bitmap_find_next_zero_area(tcs->slots, MAX_TCS_SLOTS, +i, msg->num_payload, 0); + if (slot == MAX_TCS_SLOTS) + break; + i += tcs->ncpt; + } while (slot + msg->num_payload - 1 >= i); + + if (slot == MAX_TCS_SLOTS) + return -ENOMEM; + +copy_data: + bitmap_set(tcs->slots, slot, msg->num_payload); + /* Copy the addresses of the resources over to the slots */ + for (i = 0; tcs->cmd_addr && i < msg->num_payload; i++) I don't think tcs->cmd_addr can be null, can it? Above, find_match() is already reaching through cmd_addr with enthusiasm. If kept, it could at least be moved outside of the loop. It would be NULL for ACTIVE_TCS. I can move the check outside the loop. Thanks, Lina
Re: [PATCH v2 07/10] drivers: qcom: rpmh: cache sleep/wake state requests
On Wed, Feb 21 2018 at 22:07 +, Evan Green wrote: Hi Lina, On Thu, Feb 15, 2018 at 9:35 AM, Lina Iyer wrote: [...] +static struct cache_req *cache_rpm_request(struct rpmh_client *rc, + enum rpmh_state state, + struct tcs_cmd *cmd) +{ + struct cache_req *req; + struct rpmh_ctrlr *rpm = rc->ctrlr; + unsigned long flags; + + spin_lock_irqsave(&rpm->lock, flags); + req = __find_req(rc, cmd->addr); + if (req) + goto existing; + + req = kzalloc(sizeof(*req), GFP_ATOMIC); + if (!req) { + req = ERR_PTR(-ENOMEM); + goto unlock; + } + + req->addr = cmd->addr; + req->sleep_val = req->wake_val = UINT_MAX; So UINT_MAX is really never a valid value to write? Maybe it would be good to at least print some sort of complaint if somebody sends down a request with this value. Otherwise the request is silently ignored and would be quite challenging to track down. Yes, UINT_MAX is a invalid value. I have not encountered anything in the spec that points to hold UINT_MAX as a valid value. It would also be quite inefficient to validate each input. Drivers generally know what they are doing. These are data sent to the hardware and there is a general understanding that incorrect data may fail silently. +/** + * rpmh_flush: Flushes the buffered active and sleep sets to TCS + * + * @rc: The RPMh handle got from rpmh_get_dev_channel + * + * This function is generally called from the sleep code from the last CPU + * that is powering down the entire system. + * + * Returns -EBUSY if the controller is busy, probably waiting on a response + * to a RPMH request sent earlier. + */ +int rpmh_flush(struct rpmh_client *rc) +{ + struct cache_req *p; + struct rpmh_ctrlr *rpm = rc->ctrlr; + int ret; + unsigned long flags; + + if (IS_ERR_OR_NULL(rc)) + return -EINVAL; + + spin_lock_irqsave(&rpm->lock, flags); + if (!rpm->dirty) { + pr_debug("Skipping flush, TCS has latest data.\n"); + spin_unlock_irqrestore(&rpm->lock, flags); + return 0; + } + spin_unlock_irqrestore(&rpm->lock, flags); + + /* +* Nobody else should be calling this function other than system PM,, +* hence we can run without locks. +*/ + list_for_each_entry(p, &rc->ctrlr->cache, list) { + if (!is_req_valid(p)) { + pr_debug("%s: skipping RPMH req: a:0x%x s:0x%x w:0x%x", + __func__, p->addr, p->sleep_val, p->wake_val); + continue; + } + ret = send_single(rc, RPMH_SLEEP_STATE, p->addr, p->sleep_val); + if (ret) + return ret; + ret = send_single(rc, RPMH_WAKE_ONLY_STATE, p->addr, + p->wake_val); + if (ret) + return ret; + } + + spin_lock_irqsave(&rpm->lock, flags); + rpm->dirty = false; + spin_unlock_irqrestore(&rpm->lock, flags); + I've got some questions on the locking in this function. I understand that the lock protects the list, and I'm surmising that you don't want to hold the lock across send_single (even though there's another lock in there that's held for most of that time, so I think you could). I'm still a newbie to Linux in general, so I'll pose this as a question: is it generally okay in Linux to traverse across a list that may have items concurrently added to it? You're never removing items from this list, so I think there are no actual bugs, but it does seem like it relies on the implementation details of the list. And if you ever did remove items from the list, this would bite you. It is generally not advisable to traverse a list if the list is being modified externally. The reason why it is okay here, is the context of this function call. The only caller of this function will be a system PM driver. As noted in the comments, this is called when the last CPU is powering down and in such a context, there are no other active users of this library. Also, why do you need to acquire the lock just to set dirty to false? Right now it looks like there's a race where someone could add an element to this list just after you've terminated this loop (but before you have the lock), but then the dirty = false here clobbers their dirty = true, and the item is never sent during future flushes. I think it would be safer and faster to set dirty = false before iterating through the list (either within the lock or outside of it given that this is the only place that reads or clears dirty). That way if new
Re: [PATCH v2 09/10] drivers: qcom: rpmh: add support for batch RPMH request
On Wed, Feb 21 2018 at 23:25 +, Evan Green wrote: Hello Lina, On Thu, Feb 15, 2018 at 9:35 AM, Lina Iyer wrote: Platform drivers need make a lot of resource state requests at the same time, say, at the start or end of an usecase. It can be quite inefficient to send each request separately. Instead they can give the RPMH library a batch of requests to be sent and wait on the whole transaction to be complete. rpmh_write_batch() is a blocking call that can be used to send multiple RPMH command sets. Each RPMH command set is set asynchronously and the API blocks until all the command sets are complete and receive their tx_done callbacks. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh.c | 150 include/soc/qcom/rpmh.h | 8 +++ 2 files changed, 158 insertions(+) diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index dff4c46be3af..6f60bb9a4dfa 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c [...] @@ -394,6 +537,11 @@ int rpmh_flush(struct rpmh_client *rc) } spin_unlock_irqrestore(&rpm->lock, flags); + /* First flush the cached batch requests */ + ret = flush_batch(rc); + if (ret) + return ret; + /* * Nobody else should be calling this function other than system PM,, * hence we can run without locks. This comment and the comment in the header of this function. @@ -438,6 +586,8 @@ int rpmh_invalidate(struct rpmh_client *rc) if (IS_ERR_OR_NULL(rc)) return -EINVAL; + invalidate_batch(rc); + Similarly to my comments in patch 7, aren't there races here with adding new elements? After flush_batch, but before invalidate_batch, somebody could call cache_batch, which would add new things on the end of the array. These new items would be clobbered by invalidate_batch, without having been flushed first. Flush is a system PM function and is not called by drivers and is not expected to run conncurrently with other threads using the RPMH library. Thanks, Lina
[PATCH v2 09/10] drivers: qcom: rpmh: add support for batch RPMH request
Platform drivers need make a lot of resource state requests at the same time, say, at the start or end of an usecase. It can be quite inefficient to send each request separately. Instead they can give the RPMH library a batch of requests to be sent and wait on the whole transaction to be complete. rpmh_write_batch() is a blocking call that can be used to send multiple RPMH command sets. Each RPMH command set is set asynchronously and the API blocks until all the command sets are complete and receive their tx_done callbacks. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh.c | 150 include/soc/qcom/rpmh.h | 8 +++ 2 files changed, 158 insertions(+) diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index dff4c46be3af..6f60bb9a4dfa 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -22,6 +22,7 @@ #define RPMH_MAX_MBOXES2 #define RPMH_TIMEOUT (10 * HZ) +#define RPMH_MAX_REQ_IN_BATCH 10 #define DEFINE_RPMH_MSG_ONSTACK(rc, s, q, c, name) \ struct rpmh_request name = {\ @@ -81,12 +82,14 @@ struct rpmh_request { * @cache: the list of cached requests * @lock: synchronize access to the controller data * @dirty: was the cache updated since flush + * @batch_cache: Cache sleep and wake requests sent as batch */ struct rpmh_ctrlr { struct rsc_drv *drv; struct list_head cache; spinlock_t lock; bool dirty; + struct rpmh_request *batch_cache[2 * RPMH_MAX_REQ_IN_BATCH]; }; /** @@ -343,6 +346,146 @@ int rpmh_write(struct rpmh_client *rc, enum rpmh_state state, } EXPORT_SYMBOL(rpmh_write); +static int cache_batch(struct rpmh_client *rc, + struct rpmh_request **rpm_msg, int count) +{ + struct rpmh_ctrlr *rpm = rc->ctrlr; + unsigned long flags; + int ret = 0; + int index = 0; + int i; + + spin_lock_irqsave(&rpm->lock, flags); + while (rpm->batch_cache[index]) + index++; + if (index + count >= 2 * RPMH_MAX_REQ_IN_BATCH) { + ret = -ENOMEM; + goto fail; + } + + for (i = 0; i < count; i++) + rpm->batch_cache[index + i] = rpm_msg[i]; +fail: + spin_unlock_irqrestore(&rpm->lock, flags); + + return ret; +} + +static int flush_batch(struct rpmh_client *rc) +{ + struct rpmh_ctrlr *rpm = rc->ctrlr; + struct rpmh_request *rpm_msg; + unsigned long flags; + int ret = 0; + int i; + + /* Send Sleep/Wake requests to the controller, expect no response */ + spin_lock_irqsave(&rpm->lock, flags); + for (i = 0; rpm->batch_cache[i]; i++) { + rpm_msg = rpm->batch_cache[i]; + ret = rpmh_rsc_write_ctrl_data(rc->ctrlr->drv, &rpm_msg->msg); + if (ret) + break; + } + spin_unlock_irqrestore(&rpm->lock, flags); + + return ret; +} + +static void invalidate_batch(struct rpmh_client *rc) +{ + struct rpmh_ctrlr *rpm = rc->ctrlr; + unsigned long flags; + int index = 0; + int i; + + spin_lock_irqsave(&rpm->lock, flags); + while (rpm->batch_cache[index]) + index++; + for (i = 0; i < index; i++) { + kfree(rpm->batch_cache[i]->free); + rpm->batch_cache[i] = NULL; + } + spin_unlock_irqrestore(&rpm->lock, flags); +} + +/** + * rpmh_write_batch: Write multiple sets of RPMH commands and wait for the + * batch to finish. + * + * @rc: The RPMh handle got from rpmh_get_dev_channel + * @state: Active/sleep set + * @cmd: The payload data + * @n: The array of count of elements in each batch, 0 terminated. + * + * Write a request to the mailbox controller without caching. If the request + * state is ACTIVE, then the requests are treated as completion request + * and sent to the controller immediately. The function waits until all the + * commands are complete. If the request was to SLEEP or WAKE_ONLY, then the + * request is sent as fire-n-forget and no ack is expected. + * + * May sleep. Do not call from atomic contexts for ACTIVE_ONLY requests. + */ +int rpmh_write_batch(struct rpmh_client *rc, enum rpmh_state state, + struct tcs_cmd *cmd, int *n) +{ + struct rpmh_request *rpm_msg[RPMH_MAX_REQ_IN_BATCH] = { NULL }; + DECLARE_COMPLETION_ONSTACK(compl); + atomic_t wait_count = ATOMIC_INIT(0); /* overwritten */ + int count = 0; + int ret, i, j; + + if (IS_ERR_OR_NULL(rc) || !cmd || !n) + return -EINVAL; + + while (n[count++] > 0) + ; + count--; + if (!count || count > RPMH_MAX_REQ_IN_BATCH) + return -EINVAL; + + /* Create async request batches */ +
[PATCH v2 06/10] drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS
Allow sleep and wake commands to be cleared from the respective TCSes, so that they can be re-populated. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh-internal.h | 1 + drivers/soc/qcom/rpmh-rsc.c | 46 2 files changed, 47 insertions(+) diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index 957b1a79c4c8..fa1d36f796b0 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -89,6 +89,7 @@ struct rsc_drv { int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg); int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, struct tcs_request *msg); +int rpmh_rsc_invalidate(struct rsc_drv *drv); void rpmh_tx_done(struct tcs_request *msg, int r); diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index ae555fdc3ab7..951b797a8cc7 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -170,6 +170,52 @@ static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) return tcs; } +/** + * rpmh_rsc_invalidate - Invalidate sleep and wake TCSes + * + * @drv: the mailbox controller + */ +int rpmh_rsc_invalidate(struct rsc_drv *drv) +{ + struct tcs_group *tcs; + int m, type, ret = 0; + int inv_types[] = { WAKE_TCS, SLEEP_TCS }; + unsigned long drv_flags, flags; + + /* Lock the DRV and clear sleep and wake TCSes */ + spin_lock_irqsave(&drv->drv_lock, drv_flags); + for (type = 0; type < ARRAY_SIZE(inv_types); type++) { + tcs = get_tcs_of_type(drv, inv_types[type]); + if (IS_ERR(tcs)) + continue; + + spin_lock_irqsave(&tcs->tcs_lock, flags); + if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { + spin_unlock_irqrestore(&tcs->tcs_lock, flags); + continue; + } + + /* Clear the enable register for each TCS of the type */ + for (m = tcs->tcs_offset; + m < tcs->tcs_offset + tcs->num_tcs; m++) { + if (!tcs_is_free(drv, m)) { + spin_unlock_irqrestore(&tcs->tcs_lock, flags); + ret = -EAGAIN; + goto drv_unlock; + } + write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0, 0); + /* Mark the TCS slots as free */ + bitmap_zero(tcs->slots, MAX_TCS_SLOTS); + } + spin_unlock_irqrestore(&tcs->tcs_lock, flags); + } +drv_unlock: + spin_unlock_irqrestore(&drv->drv_lock, drv_flags); + + return ret; +} +EXPORT_SYMBOL(rpmh_rsc_invalidate); + static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, struct tcs_request *msg) { -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v2 00/10] drivers/qcom: add RPMH communication support
Changes in v2: - Added sleep/wake, async and batch requests support - Addressed Bjorn's comments - Private FTRACE for drivers/soc/qcom as suggested by Steven - Sparse checked on these patches - Use SPDX license commenting sytle This set of patches add the ability for platform drivers to make use of shared resources in newer Qualcomm SoCs like SDM845. Resources that are shared between multiple processors in a SoC are generally controlled by a dedicated remote processor. The remote processor (Resource Power Manager or RPM in previous QCOM SoCs) receives requests for resource state from other processors using the shared resource, aggregates the request and applies the result on the shared resource. SDM845 advances this concept and uses h/w (hardened I/P) blocks for aggregating requests and applying the result on the resource. The resources could be clocks, regulators or bandwidth requests for buses. This new architecture is called RPM-hardened or RPMH in short. Since this communication mechanism is completely hardware driven without a processor intervention on the remote end, existing mechanisms like RPM-SMD are no longer useful. Also, there is no serialization of data or is data is written to a shared memory in this new format. The data used is different, unsigned 32 bits are used for representing an address, data and header. Each resource's property is a unique u32 address and have pre-defined set of property specific valid values. A request that comprises of is sent by writing to a set of registers from Linux and transmitted to the remote slave through an internal bus. The remote end aggregates this request along with requests from other processors for the and applies the result. The hardware block that houses this functionality is called Resource State Coordinator or RSC. Inside the RSC are set of slots for sending RPMH requests called Trigger Commands Sets (TCS). The set of patches are for writing the requests into these TCSes and sending them to hardened IP blocks. The driver design is split into two components. The RSC driver housed in rpmh-rsc.c and the set of library functions in rpmh.c that frame the request and transmit it using the controller. This first set of patches allow a simple synchronous request to be made by the platform drivers. Future patches will add more functionality that cater to complex drivers and use cases. Please consider reviewing this patchset. v1: https://www.spinics.net/lists/devicetree/msg210980.html Lina Iyer (10): drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE drivers: qcom: rpmh: add RPMH helper functions drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS drivers: qcom: rpmh-rsc: allow invalidation of sleep/wake TCS drivers: qcom: rpmh: cache sleep/wake state requests drivers: qcom: rpmh: allow requests to be sent asynchronously drivers: qcom: rpmh: add support for batch RPMH request drivers: qcom: rpmh-rsc: allow active requests from wake TCS .../devicetree/bindings/arm/msm/rpmh-rsc.txt | 146 drivers/soc/qcom/Kconfig | 10 + drivers/soc/qcom/Makefile | 4 + drivers/soc/qcom/rpmh-internal.h | 96 +++ drivers/soc/qcom/rpmh-rsc.c| 794 + drivers/soc/qcom/rpmh.c| 670 + drivers/soc/qcom/trace-rpmh.h | 89 +++ include/dt-bindings/soc/qcom,rpmh-rsc.h| 14 + include/soc/qcom/rpmh.h| 60 ++ include/soc/qcom/tcs.h | 56 ++ 10 files changed, 1939 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt create mode 100644 drivers/soc/qcom/rpmh-internal.h create mode 100644 drivers/soc/qcom/rpmh-rsc.c create mode 100644 drivers/soc/qcom/rpmh.c create mode 100644 drivers/soc/qcom/trace-rpmh.h create mode 100644 include/dt-bindings/soc/qcom,rpmh-rsc.h create mode 100644 include/soc/qcom/rpmh.h create mode 100644 include/soc/qcom/tcs.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v2 10/10] drivers: qcom: rpmh-rsc: allow active requests from wake TCS
Some RSCs may only have sleep and wake TCS, i.e, there is no dedicated TCS for active mode request, but drivers may still want to make active requests from these RSCs. In such cases re-purpose the wake TCS to send active state requests. The requirement for this is that the driver is aware that the wake TCS is being repurposed to send active request, hence the sleep and wake TCSes be invalidated before the active request is sent. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh-rsc.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 951b797a8cc7..e13abf9ad8fd 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -220,6 +220,7 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, struct tcs_request *msg) { int type; + struct tcs_group *tcs; switch (msg->state) { case RPMH_ACTIVE_ONLY_STATE: @@ -235,7 +236,22 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, return ERR_PTR(-EINVAL); } - return get_tcs_of_type(drv, type); + /* +* If we are making an active request on a RSC that does not have a +* dedicated TCS for active state use, then re-purpose a wake TCS to +* send active votes. +* NOTE: The driver must be aware that this RSC does not have a +* dedicated AMC, and therefore would invalidate the sleep and wake +* TCSes before making an active state request. +*/ + tcs = get_tcs_of_type(drv, type); + if (msg->state == RPMH_ACTIVE_ONLY_STATE && IS_ERR(tcs)) { + tcs = get_tcs_of_type(drv, WAKE_TCS); + if (!IS_ERR(tcs)) + rpmh_rsc_invalidate(drv); + } + + return tcs; } static void send_tcs_response(struct tcs_response *resp) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v2 03/10] drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE
Log sent RPMH requests and interrupt responses in FTRACE. Cc: Steven Rostedt Signed-off-by: Lina Iyer --- drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/rpmh-rsc.c | 6 +++ drivers/soc/qcom/trace-rpmh.h | 89 +++ 3 files changed, 96 insertions(+) create mode 100644 drivers/soc/qcom/trace-rpmh.h diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 7426979ea9fb..094b639ece2f 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +CFLAGS_trace-rpmh.o := -I$(src) obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o obj-$(CONFIG_QCOM_GSBI)+= qcom_gsbi.o obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index ceb8aea237d6..58a3c53d651e 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -24,6 +24,9 @@ #include "rpmh-internal.h" +#define CREATE_TRACE_POINTS +#include "trace-rpmh.h" + #define RSC_DRV_TCS_OFFSET 672 #define RSC_DRV_CMD_OFFSET 20 @@ -239,6 +242,7 @@ static irqreturn_t tcs_irq_handler(int irq, void *p) /* Reclaim the TCS */ write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0, 0); write_tcs_reg(drv, RSC_DRV_IRQ_CLEAR, 0, 0, BIT(m)); + trace_rpmh_notify_irq(drv, resp); atomic_set(&drv->tcs_in_use[m], 0); send_tcs_response(resp); } @@ -270,6 +274,7 @@ static void tcs_notify_tx_done(unsigned long data) struct tcs_response, list); list_del(&resp->list); spin_unlock_irqrestore(&drv->drv_lock, flags); + trace_rpmh_notify_tx_done(drv, resp); free_response(resp); } } @@ -298,6 +303,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int m, int n, write_tcs_reg(drv, RSC_DRV_CMD_MSGID, m, n + i, msgid); write_tcs_reg(drv, RSC_DRV_CMD_ADDR, m, n + i, cmd->addr); write_tcs_reg(drv, RSC_DRV_CMD_DATA, m, n + i, cmd->data); + trace_rpmh_send_msg(drv, m, n + i, msgid, cmd); } write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0, cmd_complete); diff --git a/drivers/soc/qcom/trace-rpmh.h b/drivers/soc/qcom/trace-rpmh.h new file mode 100644 index ..734de0d54a9f --- /dev/null +++ b/drivers/soc/qcom/trace-rpmh.h @@ -0,0 +1,89 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + */ + +#if !defined(_TRACE_RPMH_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_RPMH_H + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM rpmh + +#include +#include "rpmh-internal.h" + +DECLARE_EVENT_CLASS(rpmh_notify, + + TP_PROTO(struct rsc_drv *d, struct tcs_response *r), + + TP_ARGS(d, r), + + TP_STRUCT__entry( + __field(const char *, d->name) + __field(int, m) + __field(u32, addr) + __field(int, errno) + ), + + TP_fast_assign( + __entry->name = d->name; + __entry->m = r->m; + __entry->addr = r->msg->payload[0].addr; + __entry->errno = r->err; + ), + + TP_printk("%s: ack: tcs-m:%d addr: 0x%08x errno: %d", +__entry->name, __entry->m, __entry->addr, __entry->errno) +); + +DEFINE_EVENT(rpmh_notify, rpmh_notify_irq, + TP_PROTO(struct rsc_drv *d, struct tcs_response *r), + TP_ARGS(d, r) +); + +DEFINE_EVENT(rpmh_notify, rpmh_notify_tx_done, + TP_PROTO(struct rsc_drv *d, struct tcs_response *r), + TP_ARGS(d, r) +); + + +TRACE_EVENT(rpmh_send_msg, + + TP_PROTO(struct rsc_drv *d, int m, int n, u32 h, struct tcs_cmd *c), + + TP_ARGS(d, m, n, h, c), + + TP_STRUCT__entry( + __field(const char*, d->name) + __field(int, m) + __field(int, n) + __field(u32, hdr) + __field(u32, addr) + __field(u32, data) + __field(bool, complete) + ), + + TP_fast_assign( + __entry->name = s; + __entry->m = m; + __entry->n = n; + __entry->hdr = h; + __entry->addr = c->addr; + __entry->data = c->data; + __entry->complete = c->complete; + ), + + TP_printk("%s: send-msg: tcs(m): %d cmd(n): %d msgid: 0x%08x addr: 0x%08x data: 0x%08x complete: %d", + __entry->name, __entry->m, __entry->n, __entry->hdr, + __entry->addr, __entry->data, __entry->complete) +); + +#endif /* _TRACE_RPMH_H */ + +#undef TRACE
[PATCH v2 08/10] drivers: qcom: rpmh: allow requests to be sent asynchronously
Platform drivers that want to send a request but do not want to block until the RPMH request completes have now a new API - rpmh_write_async(). The API allocates memory and send the requests and returns the control back to the platform driver. The tx_done callback from the controller is handled in the context of the controller's thread and frees the allocated memory. This API allows RPMH requests from atomic contexts as well. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh.c | 52 + include/soc/qcom/rpmh.h | 8 2 files changed, 60 insertions(+) diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index 671bc03ad77a..dff4c46be3af 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -35,6 +35,7 @@ .completion = q,\ .wait_count = c,\ .rc = rc, \ + .free = NULL, \ } @@ -61,6 +62,7 @@ struct cache_req { * @completion: triggered when request is done * @wait_count: count of waiters for this completion * @err: err return from the controller + * @free: the request object to be freed at tx_done */ struct rpmh_request { struct tcs_request msg; @@ -69,6 +71,7 @@ struct rpmh_request { atomic_t *wait_count; struct rpmh_client *rc; int err; + struct rpmh_request *free; }; /** @@ -114,6 +117,8 @@ void rpmh_tx_done(struct tcs_request *msg, int r) "RPMH TX fail in msg addr 0x%x, err=%d\n", rpm_msg->msg.payload[0].addr, r); + kfree(rpm_msg->free); + /* Signal the blocking thread we are done */ if (wc && atomic_dec_and_test(wc) && compl) complete(compl); @@ -257,6 +262,53 @@ static int __rpmh_write(struct rpmh_client *rc, enum rpmh_state state, return ret; } +static struct rpmh_request *__get_rpmh_msg_async(struct rpmh_client *rc, + enum rpmh_state state, + struct tcs_cmd *cmd, int n) +{ + struct rpmh_request *req; + + if (IS_ERR_OR_NULL(rc) || !cmd || n <= 0 || n > MAX_RPMH_PAYLOAD) + return ERR_PTR(-EINVAL); + + req = kcalloc(1, sizeof(*req), GFP_ATOMIC); + if (!req) + return ERR_PTR(-ENOMEM); + + memcpy(req->cmd, cmd, n * sizeof(*cmd)); + + req->msg.state = state; + req->msg.payload = req->cmd; + req->msg.num_payload = n; + req->free = req; + + return req; +} + +/** + * rpmh_write_async: Write a set of RPMH commands + * + * @rc: The RPMh handle got from rpmh_get_dev_channel + * @state: Active/sleep set + * @cmd: The payload data + * @n: The number of elements in payload + * + * Write a set of RPMH commands, the order of commands is maintained + * and will be sent as a single shot. + */ +int rpmh_write_async(struct rpmh_client *rc, enum rpmh_state state, + struct tcs_cmd *cmd, int n) +{ + struct rpmh_request *rpm_msg; + + rpm_msg = __get_rpmh_msg_async(rc, state, cmd, n); + if (IS_ERR(rpm_msg)) + return PTR_ERR(rpm_msg); + + return __rpmh_write(rc, state, rpm_msg); +} +EXPORT_SYMBOL(rpmh_write_async); + /** * rpmh_write: Write a set of RPMH commands and block until response * diff --git a/include/soc/qcom/rpmh.h b/include/soc/qcom/rpmh.h index a3f1246ce49a..172a649f1a1c 100644 --- a/include/soc/qcom/rpmh.h +++ b/include/soc/qcom/rpmh.h @@ -15,6 +15,9 @@ struct rpmh_client; int rpmh_write(struct rpmh_client *rc, enum rpmh_state state, struct tcs_cmd *cmd, int n); +int rpmh_write_async(struct rpmh_client *rc, enum rpmh_state state, + struct tcs_cmd *cmd, int n); + struct rpmh_client *rpmh_get_client(struct platform_device *pdev); int rpmh_flush(struct rpmh_client *rc); @@ -32,6 +35,11 @@ static inline int rpmh_write(struct rpmh_client *rc, enum rpmh_state state, static inline struct rpmh_client *rpmh_get_client(struct platform_device *pdev) { return ERR_PTR(-ENODEV); } +static inline int rpmh_write_async(struct rpmh_client *rc, + enum rpmh_state state, + struct tcs_cmd *cmd, int n) +{ return -ENODEV; } + static inline int rpmh_flush(struct rpmh_client *rc) { return -ENODEV; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v2 07/10] drivers: qcom: rpmh: cache sleep/wake state requests
Active state requests are sent immediately to the mailbox controller, while sleep and wake state requests are cached in this driver to avoid taxing the mailbox controller repeatedly. The cached values will be sent to the controller when the rpmh_flush() is called. Generally, flushing is a system PM activity and may be called from the system PM drivers when the system is entering suspend or deeper sleep modes during cpuidle. Also allow invalidating the cached requests, so they may be re-populated again. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh.c | 213 +++- include/soc/qcom/rpmh.h | 10 +++ 2 files changed, 222 insertions(+), 1 deletion(-) diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c index d95ea3fa8b67..671bc03ad77a 100644 --- a/drivers/soc/qcom/rpmh.c +++ b/drivers/soc/qcom/rpmh.c @@ -6,11 +6,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include @@ -35,6 +37,22 @@ .rc = rc, \ } + +/** + * cache_req : the request object for caching + * + * @addr: the address of the resource + * @sleep_val: the sleep vote + * @wake_val: the wake vote + * @list: linked list obj + */ +struct cache_req { + u32 addr; + u32 sleep_val; + u32 wake_val; + struct list_head list; +}; + /** * rpmh_request: the message to be sent to rpmh-rsc * @@ -57,9 +75,15 @@ struct rpmh_request { * rpmh_ctrlr: our representation of the controller * * @drv: the controller instance + * @cache: the list of cached requests + * @lock: synchronize access to the controller data + * @dirty: was the cache updated since flush */ struct rpmh_ctrlr { struct rsc_drv *drv; + struct list_head cache; + spinlock_t lock; + bool dirty; }; /** @@ -123,17 +147,91 @@ static int wait_for_tx_done(struct rpmh_client *rc, return (ret > 0) ? 0 : -ETIMEDOUT; } +static struct cache_req *__find_req(struct rpmh_client *rc, u32 addr) +{ + struct cache_req *p, *req = NULL; + + list_for_each_entry(p, &rc->ctrlr->cache, list) { + if (p->addr == addr) { + req = p; + break; + } + } + + return req; +} + +static struct cache_req *cache_rpm_request(struct rpmh_client *rc, + enum rpmh_state state, + struct tcs_cmd *cmd) +{ + struct cache_req *req; + struct rpmh_ctrlr *rpm = rc->ctrlr; + unsigned long flags; + + spin_lock_irqsave(&rpm->lock, flags); + req = __find_req(rc, cmd->addr); + if (req) + goto existing; + + req = kzalloc(sizeof(*req), GFP_ATOMIC); + if (!req) { + req = ERR_PTR(-ENOMEM); + goto unlock; + } + + req->addr = cmd->addr; + req->sleep_val = req->wake_val = UINT_MAX; + INIT_LIST_HEAD(&req->list); + list_add_tail(&req->list, &rpm->cache); + +existing: + switch (state) { + case RPMH_ACTIVE_ONLY_STATE: + if (req->sleep_val != UINT_MAX) + req->wake_val = cmd->data; + break; + case RPMH_WAKE_ONLY_STATE: + req->wake_val = cmd->data; + break; + case RPMH_SLEEP_STATE: + req->sleep_val = cmd->data; + break; + default: + break; + }; + + rpm->dirty = true; +unlock: + spin_unlock_irqrestore(&rpm->lock, flags); + + return req; +} + /** - * __rpmh_write: send the RPMH request + * __rpmh_write: Cache and send the RPMH request * * @rc: The RPMH client * @state: Active/Sleep request type * @rpm_msg: The data that needs to be sent (payload). + * + * Cache the RPMH request and send if the state is ACTIVE_ONLY. + * SLEEP/WAKE_ONLY requests are not sent to the controller at + * this time. Use rpmh_flush() to send them to the controller. */ static int __rpmh_write(struct rpmh_client *rc, enum rpmh_state state, struct rpmh_request *rpm_msg) { int ret = -EFAULT; + struct cache_req *req; + int i; + + /* Cache the request in our store and link the payload */ + for (i = 0; i < rpm_msg->msg.num_payload; i++) { + req = cache_rpm_request(rc, state, &rpm_msg->msg.payload[i]); + if (IS_ERR(req)) + return PTR_ERR(req); + } rpm_msg->msg.state = state; @@ -150,6 +248,10 @@ static int __rpmh_write(struct rpmh_client *rc, enum rpmh_state state, "Error in RPMH request addr=0x%x, data=0x%x\n", rpm_msg->msg.payload[0].addr,
[PATCH v2 02/10] dt-bindings: introduce RPMH RSC bindings for Qualcomm SoCs
Add device binding documentation for Qualcomm Technology Inc's RPMH RSC driver. The hardware block is used for communicating resource state requests for shared resources. Cc: devicet...@vger.kernel.org Signed-off-by: Lina Iyer --- .../devicetree/bindings/arm/msm/rpmh-rsc.txt | 146 + 1 file changed, 146 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt diff --git a/Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt b/Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt new file mode 100644 index ..6cb36ae98bf9 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/rpmh-rsc.txt @@ -0,0 +1,146 @@ +RPMH RSC: + + +RPMH is the mechanism for communicating with the hardened resource +accelerators. Requests to the resources can be written to the TCS mailbox +registers and using a (addr, val) pair and triggered. Messages in the TCS are +then sent in sequence over an internal bus. + +The hardware block (Direct Resource Voter or DRV) is a part of the h/w entity +(Resource State Coordinator a.k.a RSC) that can handle a multiple sleep and +active/wake resource requests. Multiple such DRVs can exist in a SoC and can +be written to from Linux. The structure of each DRV follows the same template +with a few variations that are captured by the properties here. + +Each DRV could have 'm' TCS instances. Each TCS could have 'n' slots. Each +slot has a header (u32), address (u32), data (u32), status (u32) and a +read-response (u32). A TCS when triggered will send all the enabled commands +of the 'n' commands in that slot in sequence. + +A TCS may be triggered from Linux or triggered by the F/W after all the CPUs +have powered off to facilitate idle power saving. TCS could be classified as - + + SLEEP, /* Triggered by F/W */ + WAKE, /* Triggered by F/W */ + ACTIVE, /* Triggered by Linux */ + CONTROL /* Triggered by F/W */ + +The order in which they are described in the DT, should match the hardware +configuration. + +Requests can be made for the state of a resource, when the subsystem is active +or idle. When all subsystems like Modem, GPU, CPU are idle, the resource state +will be an aggregate of the sleep votes from each of those subsystem. Drivers +may request a sleep value for their shared resources in addition to the active +mode requests. + +Control requests are instance specific requests that may or may not reach an +accelerator. Only one platform device in Linux can request a control channel +on a DRV. + +CONTROLLER: +-- + +PROPERTIES: + +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,rpmh-rsc". + +- reg: + Usage: required + Value type: + Definition: The first register specifies the base address of the DRV. + The second register specifies the start address of the + TCS. + +- reg-names: + Usage: required + Value type: + Definition: Maps the register specified in the reg property. Must be + "drv" and "tcs". + +- interrupts: + Usage: required + Value type: + Definition: The interrupt that trips when a message complete/response + is received for this DRV from the accelerators. + +- qcom,drv-id: + Usage: required + Value type: + Definition: the id of the DRV in the RSC block. + +- qcom,tcs-config: + Usage: required + Value type: + Definition: the tuple defining the configuration of TCS. + Must have 2 cells which describe each TCS type. + . + The order of the TCS must match the hardware + configuration. + - Cell #1 (TCS Type): TCS types to be specified - + SLEEP_TCS + WAKE_TCS + ACTIVE_TCS + CONTROL_TCS + - Cell #2 (Number of TCS): + +- label: + Usage: optional + Value type: + Definition: Name for the RSC. The name would be used in trace logs. + +Platform drivers that want to use the RSC to communicate with RPMH must +specify their bindings as child of the corresponding RSC controllers. + +EXAMPLE 1: + +For a TCS whose RSC base address is is 0x179C and is at a DRV id of 2, the +register offsets for DRV2 start at 0D00, the register calculations are like +this - +First tuple: 0x179C + 0x1 * 2 = 0x179E +Second tuple: 0x179E + 0xD00 = 0x179E0D00 + + apps_rsc: rsc@179e000 { + label = "apps_rsc"; + compatible = "qcom,rpmh-rsc"; + reg = <0x179e 0x1>, <0x179e0d00 0x3000>; + reg-names = "drv", "tcs"; + interrupts = + qcom,drv-id = <2>; + qcom,tcs-config = , +
[PATCH v2 01/10] drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs
Add controller driver for QCOM SoCs that have hardware based shared resource management. The hardware IP known as RSC (Resource State Coordinator) houses multiple Direct Resource Voter (DRV) for different execution levels. A DRV is a unique voter on the state of a shared resource. A Trigger Control Set (TCS) is a bunch of slots that can house multiple resource state requests, that when triggered will issue those requests through an internal bus to the Resource Power Manager Hardened (RPMH) blocks. These hardware blocks are capable of adjusting clocks, voltages etc. The resource state request from a DRV are aggregated along with state requests from other processors in the SoC and the aggregate value is applied on the resource. Some important aspects of the RPMH communication - - Requests are with some header information - Multiple requests (upto 16) may be sent through a TCS, at a time - Requests in a TCS are sent in sequence - Requests may be fire-n-forget or completion (response expected) - Multiple TCS from the same DRV may be triggered simultaneously - Cannot send a request if another reques for the same addr is in progress from the same DRV - When all the requests from a TCS are complete, an IRQ is raised - The IRQ handler needs to clear the TCS before it is available for reuse - TCS configuration is specific to a DRV - Platform drivers may use DRV from different RSCs to make requests Resource state requests made when CPUs are active are called 'active' state requests. Requests made when all the CPUs are powered down (idle state) are called 'sleep' state requests. They are matched by a corresponding 'wake' state requests which puts the resources back in to previously requested active state before resuming any CPU. TCSes are dedicated for each type of requests. Control TCS are used to provide specific information to the controller. Signed-off-by: Lina Iyer --- drivers/soc/qcom/Kconfig| 10 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/rpmh-internal.h| 86 + drivers/soc/qcom/rpmh-rsc.c | 593 include/dt-bindings/soc/qcom,rpmh-rsc.h | 14 + include/soc/qcom/tcs.h | 56 +++ 6 files changed, 760 insertions(+) create mode 100644 drivers/soc/qcom/rpmh-internal.h create mode 100644 drivers/soc/qcom/rpmh-rsc.c create mode 100644 include/dt-bindings/soc/qcom,rpmh-rsc.h create mode 100644 include/soc/qcom/tcs.h diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index b81374bb6713..90b038bded0b 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -46,6 +46,16 @@ config QCOM_RMTFS_MEM Say y here if you intend to boot the modem remoteproc. +config QCOM_RPMH + bool "Qualcomm RPM-Hardened (RPMH) Communication" + depends on ARCH_QCOM && OF + help + Support for communication with the hardened-RPM blocks in + Qualcomm Technologies Inc (QTI) SoCs. RPMH communication uses an + internal bus to transmit state requests for shared resources. A set + of hardware components aggregate requests for these resources and + help apply the aggregated state on the resource. + config QCOM_SMEM tristate "Qualcomm Shared Memory Manager (SMEM)" depends on ARCH_QCOM diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 40c56f67e94a..7426979ea9fb 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -10,3 +10,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o obj-$(CONFIG_QCOM_SMP2P) += smp2p.o obj-$(CONFIG_QCOM_SMSM)+= smsm.o obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o +obj-$(CONFIG_QCOM_RPMH)+= rpmh-rsc.o diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h new file mode 100644 index ..dccf9de1a75f --- /dev/null +++ b/drivers/soc/qcom/rpmh-internal.h @@ -0,0 +1,86 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + */ + + +#ifndef __RPM_INTERNAL_H__ +#define __RPM_INTERNAL_H__ + +#include + +#define TCS_TYPE_NR4 +#define MAX_CMDS_PER_TCS 16 +#define MAX_TCS_PER_TYPE 3 +#define MAX_TCS_NR (MAX_TCS_PER_TYPE * TCS_TYPE_NR) + +struct rsc_drv; + +/** + * tcs_response: Response object for a request + * + * @drv: the controller + * @msg: the request for this response + * @m: the tcs identifier + * @err: error reported in the response + * @list: link list object. + */ +struct tcs_response { + struct rsc_drv *drv; + struct tcs_request *msg; + u32 m; + int err; + struct list_head list; +}; + +/** + * tcs_group: group of TCSes for a request state + * + * @type: type of the TCS in this group - active, sleep, wake + * @tcs_mask: mask of the TCSes relative to all the TCSes i
[PATCH v2 04/10] drivers: qcom: rpmh: add RPMH helper functions
Sending RPMH requests and waiting for response from the controller through a callback is common functionality across all platform drivers. To simplify drivers, add a library functions to create RPMH client and send resource state requests. rpmh_write() is a synchronous blocking call that can be used to send active state requests. Signed-off-by: Lina Iyer --- drivers/soc/qcom/Makefile| 4 +- drivers/soc/qcom/rpmh-internal.h | 2 + drivers/soc/qcom/rpmh-rsc.c | 7 ++ drivers/soc/qcom/rpmh.c | 257 +++ include/soc/qcom/rpmh.h | 34 ++ 5 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 drivers/soc/qcom/rpmh.c create mode 100644 include/soc/qcom/rpmh.h diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 094b639ece2f..e5a980284f89 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -11,4 +11,6 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o obj-$(CONFIG_QCOM_SMP2P) += smp2p.o obj-$(CONFIG_QCOM_SMSM)+= smsm.o obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o -obj-$(CONFIG_QCOM_RPMH)+= rpmh-rsc.o +obj-$(CONFIG_QCOM_RPMH)+= qcom_rpmh.o +qcom_rpmh-y+= rpmh-rsc.o +qcom_rpmh-y+= rpmh.o diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index dccf9de1a75f..9c6f8f5faa6a 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -83,4 +83,6 @@ struct rsc_drv { int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg); +void rpmh_tx_done(struct tcs_request *msg, int r); + #endif /* __RPM_INTERNAL_H__ */ diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 58a3c53d651e..5d022363be33 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -263,6 +263,8 @@ static void tcs_notify_tx_done(unsigned long data) struct rsc_drv *drv = (struct rsc_drv *)data; struct tcs_response *resp; unsigned long flags; + struct tcs_request *msg; + int err; for (;;) { spin_lock_irqsave(&drv->drv_lock, flags); @@ -275,7 +277,10 @@ static void tcs_notify_tx_done(unsigned long data) list_del(&resp->list); spin_unlock_irqrestore(&drv->drv_lock, flags); trace_rpmh_notify_tx_done(drv, resp); + msg = resp->msg; + err = resp->err; free_response(resp); + rpmh_tx_done(msg, err); } } @@ -575,6 +580,8 @@ static int rpmh_rsc_probe(struct platform_device *pdev) write_tcs_reg(drv, RSC_DRV_IRQ_ENABLE, 0, 0, drv->tcs[ACTIVE_TCS].tcs_mask); + dev_set_drvdata(&pdev->dev, drv); + return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev); } diff --git a/drivers/soc/qcom/rpmh.c b/drivers/soc/qcom/rpmh.c new file mode 100644 index ..d95ea3fa8b67 --- /dev/null +++ b/drivers/soc/qcom/rpmh.c @@ -0,0 +1,257 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include "rpmh-internal.h" + +#define RPMH_MAX_MBOXES2 +#define RPMH_TIMEOUT (10 * HZ) + +#define DEFINE_RPMH_MSG_ONSTACK(rc, s, q, c, name) \ + struct rpmh_request name = {\ + .msg = {\ + .state = s, \ + .payload = name.cmd,\ + .num_payload = 0, \ + .is_complete = true,\ + }, \ + .cmd = { { 0 } }, \ + .completion = q,\ + .wait_count = c,\ + .rc = rc, \ + } + +/** + * rpmh_request: the message to be sent to rpmh-rsc + * + * @msg: the request + * @cmd: the payload that will be part of the @msg + * @completion: triggered when request is done + * @wait_count: count of waiters for this completion + * @err: err return from the controller + */ +struct rpmh_request { + struct tcs_request msg; + struct tcs_cmd cmd[MAX_RPMH_PAYLOAD]; + struct completion *completion; + atomic_t *wait_count; + struct rpmh_client *rc; + int err; +}; + +/** + * rpmh_ctrlr: our representation of the controller + * + * @drv: the controller instance + */ +struct rpmh_ctrlr { + struct rsc_drv *drv; +}; + +/** + * rpmh_client: the client object + * + * @dev: the platform device that is the owner + * @ctrlr: the control
[PATCH v2 05/10] drivers: qcom: rpmh-rsc: write sleep/wake requests to TCS
Sleep and wake requests are sent when the application processor subsystem of the SoC is entering deep sleep states like in suspend. These requests help lower the system power requirements when the resources are not in use. Sleep and wake requests are written to the TCS slots but are not triggered at the time of writing. The TCS are triggered by the firmware after the last of the CPUs has executed its WFI. Since these requests may come in different batches of requests, it is job of this controller driver to find arrange the requests into the available TCSes. Signed-off-by: Lina Iyer --- drivers/soc/qcom/rpmh-internal.h | 7 +++ drivers/soc/qcom/rpmh-rsc.c | 126 +++ 2 files changed, 133 insertions(+) diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index 9c6f8f5faa6a..957b1a79c4c8 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -13,6 +13,7 @@ #define MAX_CMDS_PER_TCS 16 #define MAX_TCS_PER_TYPE 3 #define MAX_TCS_NR (MAX_TCS_PER_TYPE * TCS_TYPE_NR) +#define MAX_TCS_SLOTS (MAX_CMDS_PER_TCS * MAX_TCS_PER_TYPE) struct rsc_drv; @@ -43,6 +44,8 @@ struct tcs_response { * @ncpt: number of commands in each TCS * @tcs_lock: lock for synchronizing this TCS writes * @responses: response objects for requests sent from each TCS + * @cmd_addr: flattened cache of cmds in sleep/wake TCS + * @slots: indicates which of @cmd_addr are occupied */ struct tcs_group { struct rsc_drv *drv; @@ -53,6 +56,9 @@ struct tcs_group { int ncpt; spinlock_t tcs_lock; struct tcs_response *responses[MAX_TCS_PER_TYPE]; + u32 *cmd_addr; + DECLARE_BITMAP(slots, MAX_TCS_SLOTS); + }; /** @@ -82,6 +88,7 @@ struct rsc_drv { int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg); +int rpmh_rsc_write_ctrl_data(struct rsc_drv *drv, struct tcs_request *msg); void rpmh_tx_done(struct tcs_request *msg, int r); diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 5d022363be33..ae555fdc3ab7 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -6,6 +6,7 @@ #define pr_fmt(fmt) "%s " fmt, KBUILD_MODNAME #include +#include #include #include #include @@ -178,6 +179,12 @@ static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, case RPMH_ACTIVE_ONLY_STATE: type = ACTIVE_TCS; break; + case RPMH_WAKE_ONLY_STATE: + type = WAKE_TCS; + break; + case RPMH_SLEEP_STATE: + type = SLEEP_TCS; + break; default: return ERR_PTR(-EINVAL); } @@ -450,6 +457,112 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, struct tcs_request *msg) } EXPORT_SYMBOL(rpmh_rsc_send_data); +static int find_match(struct tcs_group *tcs, struct tcs_cmd *cmd, int len) +{ + bool found = false; + int i = 0, j; + + /* Check for already cached commands */ + while ((i = find_next_bit(tcs->slots, MAX_TCS_SLOTS, i)) < + MAX_TCS_SLOTS) { + if (tcs->cmd_addr[i] != cmd[0].addr) { + i++; + continue; + } + /* sanity check to ensure the seq is same */ + for (j = 1; j < len; j++) { + WARN((tcs->cmd_addr[i + j] != cmd[j].addr), + "Message does not match previous sequence.\n"); + return -EINVAL; + } + found = true; + break; + } + + return found ? i : -1; +} + +static int find_slots(struct tcs_group *tcs, struct tcs_request *msg, +int *m, int *n) +{ + int slot, offset; + int i = 0; + + /* Find if we already have the msg in our TCS */ + slot = find_match(tcs, msg->payload, msg->num_payload); + if (slot >= 0) + goto copy_data; + + /* Do over, until we can fit the full payload in a TCS */ + do { + slot = bitmap_find_next_zero_area(tcs->slots, MAX_TCS_SLOTS, +i, msg->num_payload, 0); + if (slot == MAX_TCS_SLOTS) + break; + i += tcs->ncpt; + } while (slot + msg->num_payload - 1 >= i); + + if (slot == MAX_TCS_SLOTS) + return -ENOMEM; + +copy_data: + bitmap_set(tcs->slots, slot, msg->num_payload); + /* Copy the addresses of the resources over to the slots */ + for (i = 0; tcs->cmd_addr && i < msg->num_payload; i++) + tcs->cmd_addr[slot + i] = msg->payload[i].addr; + + offset = slot / tcs->ncpt; + *m = offset + tcs->tcs_offset; + *n = slo
Re: [PATCH v2 2/2] dt-bindings: introduce Command DB for QCOM SoCs
On Wed, Feb 14 2018 at 19:34 +, Bjorn Andersson wrote: On Thu 08 Feb 11:51 PST 2018, Lina Iyer wrote: From: Mahesh Sivasubramanian Command DB provides information on shared resources like clocks, regulators etc., probed at boot by the remote subsytem and made available in shared memory. Cc: devicet...@vger.kernel.org Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt new file mode 100644 index ..e21666e40ebf --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt @@ -0,0 +1,38 @@ +Command DB +- + +Command DB is a database that provides a mapping between resource key and the +resource address for a system resource managed by a remote processor. The data +is stored in a shared memory region and is loaded by the remote processor. + +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for +controlling shared resources. Depending on the board configuration the shared +resource properties may change. These properties are dynamically probed by the +remote processor and made available in the shared memory. + +The devicetree representation of the command DB driver should be: + +PROPERTIES: +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,cmd-db" + +- memory-region: + Usage: required + Value type: + Definition: The phandle to the reserved memory region. + +Example: + + reserved-memory { + [...] + cmd_db_mem: qcom,cmd-db@c3f000c { + reg = <0x0 0xc3f000c 0x0 0x8>; I was hoping that we can describe the actual memory here, as I got the impression that it will also be a chunk of memory carved out from System RAM. Yes, it can be described. But, the location should always be read from the address above. Generally, the CMD DB memory would not change, but it is not always guaranteed. If not it would seem unlikely that there's a 8 byte carveout in the middle of DDR, what else is here? + }; + }; + + qcom,cmd-db@c3f000c { + compatible = "qcom,cmd-db"; Add "qcom,cmd-db" to "reserved_mem_matches" in drivers/of/platform.c, use of_reserved_mem_lookup(pdev->dev.of_node) to get the reserved_mem and you can just put the compatible directly on the reserved-memory node. That way you don't need this separate node that doesn't really represent anything. Oh okay. I can take care of that. Thanks, Lina + memory-region = <&cmd_db_mem>; + };
Re: [PATCH v6 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Mon, Feb 12 2018 at 13:40 +, Thomas Gleixner wrote: On Fri, 9 Feb 2018, Lina Iyer wrote: +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, My previous comment about using binary constants still stands. Please either address review comments or reply at least. Ignoring reviews is not an option. Aside of that I really have to ask about the naming of these constants. Are these names hardware register nomenclature? If yes, they are disgusting. If no, they are still disgusting, but should be changed to sensible ones, which just match the IRQ_TYPE naming convention. PDC_LEVEL_LOW= 000b, PDC_EDGE_FALLING = 010b, Checkpatch doesn't like binary constants. I guess I will need to keep the enum definitions in hex or decimal. I will remove the binary from the comments though. commit 95e2c6023b0e4c8499fb521697f79215f69135fe Author: Joe Perches Date: Wed Jul 3 15:05:20 2013 -0700 checkpatch: warn when using gcc's binary constant ("0b") extension The gcc extension for binary constants that start with 0b is only supported with gcc version 4.3 or higher. The kernel can still be compiled with earlier versions of gcc, so have checkpatch emit a warning for these constants. Restructure checkpatch's constant finding code a bit to support finding these binary constants. Signed-off-by: Joe Perches Suggested-by: Andrew Morton Cc: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Thanks, Lina
Re: [PATCH v6 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Thu, Feb 15 2018 at 20:24 +, Thomas Gleixner wrote: On Thu, 15 Feb 2018, Lina Iyer wrote: On Mon, Feb 12 2018 at 13:40 +, Thomas Gleixner wrote: > On Fri, 9 Feb 2018, Lina Iyer wrote: > > +enum pdc_irq_config_bits { > > + PDC_POLARITY_LOW= 0, > > + PDC_FALLING_EDGE= 2, > > + PDC_POLARITY_HIGH = 4, > > + PDC_RISING_EDGE = 6, > > + PDC_DUAL_EDGE = 7, > > My previous comment about using binary constants still stands. Please > either address review comments or reply at least. Ignoring reviews is not > an option. > > Aside of that I really have to ask about the naming of these constants. Are > these names hardware register nomenclature? If yes, they are disgusting. If > no, they are still disgusting, but should be changed to sensible ones, > which just match the IRQ_TYPE naming convention. > >PDC_LEVEL_LOW = 000b, >PDC_EDGE_FALLING= 010b, > > > Checkpatch doesn't like binary constants. I guess I will need to keep the enum definitions in hex or decimal. I will remove the binary from the comments though. Well checkpatch is not always right. commit 95e2c6023b0e4c8499fb521697f79215f69135fe Author: Joe Perches Date: Wed Jul 3 15:05:20 2013 -0700 checkpatch: warn when using gcc's binary constant ("0b") extension The gcc extension for binary constants that start with 0b is only supported with gcc version 4.3 or higher. Can anything of this be compiled with gcc < 4.3? I don't see a reason why this would be compiled with a older GCC. I am okay with ignoring the checkpatch errors. I was just not sure if I should. Thanks, Lina
Re: [PATCH v2 03/10] drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE
On Thu, Feb 15 2018 at 19:57 +, Steven Rostedt wrote: On Thu, 15 Feb 2018 10:35:00 -0700 Lina Iyer wrote: @@ -298,6 +303,7 @@ static void __tcs_buffer_write(struct rsc_drv *drv, int m, int n, write_tcs_reg(drv, RSC_DRV_CMD_MSGID, m, n + i, msgid); write_tcs_reg(drv, RSC_DRV_CMD_ADDR, m, n + i, cmd->addr); write_tcs_reg(drv, RSC_DRV_CMD_DATA, m, n + i, cmd->data); + trace_rpmh_send_msg(drv, m, n + i, msgid, cmd); No biggy, but I'm curious to why you didn't do something this: +static void __tcs_buffer_write(struct rsc_drv *drv, int m, int n, + struct tcs_request *msg) +{ + u32 msgid, cmd_msgid = 0; + u32 cmd_enable = 0; + u32 cmd_complete; + struct tcs_cmd *cmd; + int i; + + cmd_msgid = CMD_MSGID_LEN; + cmd_msgid |= (msg->is_complete) ? CMD_MSGID_RESP_REQ : 0; + cmd_msgid |= CMD_MSGID_WRITE; + + cmd_complete = read_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0); + + for (i = 0; i < msg->num_payload; i++) { int bit = n + i; + cmd = &msg->payload[i]; + cmd_enable |= BIT(bit); + cmd_complete |= cmd->complete << (n + i); + msgid = cmd_msgid; + msgid |= (cmd->complete) ? CMD_MSGID_RESP_REQ : 0; + write_tcs_reg(drv, RSC_DRV_CMD_MSGID, m, bit, msgid); + write_tcs_reg(drv, RSC_DRV_CMD_ADDR, m, bit, cmd->addr); + write_tcs_reg(drv, RSC_DRV_CMD_DATA, m, bit, cmd->data); trace_rpmh_send_msg(drv, m, bit, msgid, cmd); The compiler should optimize that, so this isn't really a big deal, but I was just curious. No particular reason. Think I just went with the logic at that time and didn't look back deeply again on the code to tidy it up. Thanks for the suggestion. + } + + write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0, cmd_complete); + cmd_enable |= read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0); + write_tcs_reg(drv, RSC_DRV_CMD_ENABLE, m, 0, cmd_enable); +} } write_tcs_reg(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0, cmd_complete); [..] +TRACE_EVENT(rpmh_send_msg, + + TP_PROTO(struct rsc_drv *d, int m, int n, u32 h, struct tcs_cmd *c), + + TP_ARGS(d, m, n, h, c), + + TP_STRUCT__entry( + __field(const char*, d->name) + __field(int, m) + __field(int, n) + __field(u32, hdr) + __field(u32, addr) + __field(u32, data) + __field(bool, complete) + ), + + TP_fast_assign( + __entry->name = s; + __entry->m = m; + __entry->n = n; + __entry->hdr = h; + __entry->addr = c->addr; + __entry->data = c->data; + __entry->complete = c->complete; + ), + + TP_printk("%s: send-msg: tcs(m): %d cmd(n): %d msgid: 0x%08x addr: 0x%08x data: 0x%08x complete: %d", + __entry->name, __entry->m, __entry->n, __entry->hdr, I'm sorry I didn't catch this in my other reviews, but please don't use direct strings in TP_printk(). In trace-cmd and perf, it has no access to that information when reading this trace event. Not to mention, if drv is freed between the time it is recorded, and the time it is read in the trace buffer, you are now referencing random memory. The way to do this in a trace event is to use the string functionality: TP_STRUCT__entry( __string(name, d->name) [..] TP_fast_assign( __assign_string(name, d->name) [..] TP_printk("%s: ...", __get_str(name), ... Then the name is recorded in the ring buffer at the time of execution of the trace event, and trace-cmd and perf can read it, and there's no worries about it being freed between recording and reading the tracing buffer. The drv would not be freed. But that said, I will use this in my patches. Thanks Steve. -- Lina
Re: [PATCH v2 03/10] drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE
Hi Steve, On Thu, Feb 15 2018 at 17:35 +, Lina Iyer wrote: Log sent RPMH requests and interrupt responses in FTRACE. Cc: Steven Rostedt Signed-off-by: Lina Iyer --- drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/rpmh-rsc.c | 6 +++ drivers/soc/qcom/trace-rpmh.h | 89 +++ 3 files changed, 96 insertions(+) create mode 100644 drivers/soc/qcom/trace-rpmh.h diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 7426979ea9fb..094b639ece2f 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +CFLAGS_trace-rpmh.o := -I$(src) I did this here.. ... +#undef TRACE_INCLUDE_PATH +#define TRACE_INCLUDE_PATH ../../drivers/soc/qcom + Yet, I had to define the whole path here for it to compile. Per comments I should just need to do this - #define TRACE_INCLUDE_PATH . But it doesn't work. What am I missing? +#undef TRACE_INCLUDE_FILE +#define TRACE_INCLUDE_FILE trace-rpmh + +#include -- Thanks, Lina
Re: [PATCH v2 03/10] drivers: qcom: rpmh-rsc: log RPMH requests in FTRACE
On Thu, Feb 15 2018 at 20:51 +, Steven Rostedt wrote: On Thu, 15 Feb 2018 20:41:18 + Lina Iyer wrote: >--- a/drivers/soc/qcom/Makefile >+++ b/drivers/soc/qcom/Makefile >@@ -1,4 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 >+CFLAGS_trace-rpmh.o := -I$(src) I did this here.. Needs to be: CFLAGS_rpmh-rsc.o You need to use the C file that has the CREATE_TRACE_POINTS define. Thank you so much. -- Lina -- Steve ... >+#undef TRACE_INCLUDE_PATH >+#define TRACE_INCLUDE_PATH ../../drivers/soc/qcom >+ Yet, I had to define the whole path here for it to compile. Per comments I should just need to do this - #define TRACE_INCLUDE_PATH . But it doesn't work. What am I missing? >+#undef TRACE_INCLUDE_FILE >+#define TRACE_INCLUDE_FILE trace-rpmh >+ >+#include >-- Thanks, Lina
[PATCH v7 0/2] irqchip: qcom: add support for PDC interrupt controller
Changes since v6: - Code clean up as suggested by Thomas - Rebased on top of 4.16-rc1 On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ RFC v2: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1600634.html v3: https://lkml.org/lkml/2018/2/6/595 v4: https://www.spinics.net/lists/linux-arm-msm/msg32906.html v5: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1605500.html v6: https://lkml.org/lkml/2018/2/9/545 Lina Iyer (2): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: describe PDC device binding .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 311 + 4 files changed, 399 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v7 2/2] dt-bindings/interrupt-controller: pdc: describe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..0b2c97ddb520 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,78 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + Must be 2. + The first element of the tuple is the PDC pin for the + interrupt. + The second element is the trigger type. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-ranges: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the starting PDC port. + The second element is the GIC hwirq number for the PDC port. + The third element is the number of interrupts in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, must do - + + wake-device { + interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>; + }; + +In this case interrupt 514 would be mapped to port 2 on the PDC as defined by +the qcom,pdc-ranges property. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v7 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 311 + 3 files changed, 321 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index d913aec85109..030c7753d45b 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -351,4 +351,13 @@ config GOLDFISH_PIC Say yes here to enable Goldfish interrupt controller driver used for Goldfish based virtual platforms. +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d27e3e3619e0..c35ee5345a53 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -85,3 +85,4 @@ obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o obj-$(CONFIG_GOLDFISH_PIC) += irq-goldfish-pic.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..fe02a59ea584 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,311 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_region { + u32 pin_base; + u32 parent_base; + u32 cnt; +}; + +static DEFINE_RAW_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; +static struct pdc_pin_region *pdc_region; +static int pdc_region_cnt; + +static void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + + index = pin_out / 32; + mask = pin_out % 32; + + raw_spin_lock(&pdc_lock); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + raw_spin_unlock(&pdc_lock); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * Level sensitive active lowLOW + * Rising edge sensitive NOT USED + * Falling edge sensitiveLOW + * Dual Edge sensitive NOT USED + * Level sensitive active High HIGH + * Falling Edge sensitiveNOT USED + * Rising edge sensitive HIGH + * Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_LEVEL_LOW = 0b000, + PDC_EDGE_FALLING= 0b010, + PDC_LEVEL_HIGH = 0b100, + PDC_EDGE_RISING = 0b110, + PDC_EDGE_DUAL = 0b111, +}; + +/** + * qcom_pdc_gic_set_type: Configure PDC for the interrupt + * + * @d: the interrupt data + * @type: the interrupt typ
[PATCH v2 1/2] drivers: qcom: add command DB driver
From: Mahesh Sivasubramanian Command DB is a simple database in the shared memory of QCOM SoCs, that provides information regarding shared resources. Some shared resources in the SoC have properties that are probed dynamically at boot by the remote processor. The information pertaining to the SoC and the platform are made available in the shared memory. Drivers can query this information using predefined strings. Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- drivers/soc/qcom/Kconfig | 9 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 321 ++ include/soc/qcom/cmd-db.h | 50 4 files changed, 381 insertions(+) create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index b81374bb6713..7facac2cae23 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -3,6 +3,15 @@ # menu "Qualcomm SoC drivers" +config QCOM_COMMAND_DB + bool "Qualcomm Command DB" + depends on ARCH_QCOM || COMPILE_TEST + help + Command DB queries shared memory by key string for shared system + resources. Platform drivers that require to set state of a shared + resource on a RPM-hardened platform must use this database to get + SoC specific identifier and information for the shared resources. + config QCOM_GLINK_SSR tristate "Qualcomm Glink SSR driver" depends on RPMSG diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index 40c56f67e94a..fdec77c26b3f 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o obj-$(CONFIG_QCOM_GSBI)+= qcom_gsbi.o obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c new file mode 100644 index ..050a56da76c8 --- /dev/null +++ b/drivers/soc/qcom/cmd-db.c @@ -0,0 +1,321 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#define NUM_PRIORITY 2 +#define MAX_SLV_ID 8 +#define CMD_DB_MAGIC 0x0C0330DBUL +#define SLAVE_ID_MASK 0x7 +#define SLAVE_ID_SHIFT 16 + +#define ENTRY_HEADER(hdr) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr->header_offset) + +#define RSC_OFFSET(hdr, ent) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr.data_offset + ent.offset) + +#define MIN(a, b) (((a) < (b)) ? (a) : (b)) + +/** + * entry_header: header for each entry in cmddb + * + * @id: resource's identifier + * @priority: unused + * @addr: the address of the resource + * @len: length of the data + * @offset: offset at which data starts + */ +struct entry_header { + u64 id; + u32 priority[NUM_PRIORITY]; + u32 addr; + u16 len; + u16 offset; +}; + +/** + * rsc_hdr: resource header information + * + * @slv_id: id for the resource + * @header_offset: Entry header offset from data + * @data_offset: Entry offset for data location + * @cnt: number of entries for HW type + * @version: MSB is major, LSB is minor + */ +struct rsc_hdr { + u16 slv_id; + u16 header_offset; + u16 data_offset; + u16 cnt; + u16 version; + u16 reserved[3]; +}; + +/** + * cmd_db_header: The DB header information + * + * @version: The cmd db version + * @magic_number: constant expected in the database + * @header: array of resources + * @check_sum: check sum for the header. Unused. + * @reserved: reserved memory + * @data: driver specific data + */ +struct cmd_db_header { + u32 version; + u32 magic_num; + struct rsc_hdr header[MAX_SLV_ID]; + u32 check_sum; + u32 reserved; + u8 data[]; +}; + +/** + * DOC: Description of the Command DB database. + * + * At the start of the command DB memory is the cmd_db_header structure. + * The cmd_db_header holds the version, checksum, magic key as well as an + * array for header for each slave (depicted by the rsc_header). Each h/w + * based accelerator is a 'slave' (shared resource) and has slave id indicating + * the type of accelerator. The rsc_header is the header for such individual + * slaves of a given type. The entries for each of these slaves begin at the + * rsc_hdr.header_offset. In addition each slave could have auxiliary data + * that may be needed by the driver. The data for the slave starts at the + * entry_header.offset to
[PATCH v2 0/2] drivers/qcom: add Command DB support
Changes in [v2]: - Use reserved memory regions instead of regs in DT - Code cleanup suggested by Bjorn and Stephen - Remove unused structures and functions. - Update function names from _get_ to _read_ - Add documentation These patches add support for reading a shared memory database in the newer QCOM SoCs called Command DB. With the new architecture on SDM845, shared resources like clocks, regulators etc., have dynamic properties. These properties may change based on external components, board configurations or available feature set. A remote processor detects these parameters and fills up the database with the resource and available state information. Platform drivers that need these shared resources will need to query this database to get the address and properties and vote for the state. The information in the database is static. The database is read-only memory location that is available for Linux. A pre-defined string is used as a key into an entry in the database. Generally, platform drivers query the database only at init to get the information they need. [v1]: https://www.spinics.net/lists/linux-arm-msm/msg32462.html Lina Iyer (2): drivers: qcom: add command DB driver dt-bindings: introduce Command DB for QCOM SoCs .../devicetree/bindings/arm/msm/cmd-db.txt | 38 +++ drivers/soc/qcom/Kconfig | 9 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 321 + include/soc/qcom/cmd-db.h | 50 5 files changed, 419 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v2 2/2] dt-bindings: introduce Command DB for QCOM SoCs
From: Mahesh Sivasubramanian Command DB provides information on shared resources like clocks, regulators etc., probed at boot by the remote subsytem and made available in shared memory. Cc: devicet...@vger.kernel.org Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt new file mode 100644 index ..e21666e40ebf --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt @@ -0,0 +1,38 @@ +Command DB +- + +Command DB is a database that provides a mapping between resource key and the +resource address for a system resource managed by a remote processor. The data +is stored in a shared memory region and is loaded by the remote processor. + +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for +controlling shared resources. Depending on the board configuration the shared +resource properties may change. These properties are dynamically probed by the +remote processor and made available in the shared memory. + +The devicetree representation of the command DB driver should be: + +PROPERTIES: +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,cmd-db" + +- memory-region: + Usage: required + Value type: + Definition: The phandle to the reserved memory region. + +Example: + + reserved-memory { + [...] + cmd_db_mem: qcom,cmd-db@c3f000c { + reg = <0x0 0xc3f000c 0x0 0x8>; + }; + }; + + qcom,cmd-db@c3f000c { + compatible = "qcom,cmd-db"; + memory-region = <&cmd_db_mem>; + }; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/2] drivers: qcom: add command DB driver
On Thu, Feb 08 2018 at 20:48 +, Jordan Crouse wrote: On Thu, Feb 08, 2018 at 12:51:53PM -0700, Lina Iyer wrote: From: Mahesh Sivasubramanian Command DB is a simple database in the shared memory of QCOM SoCs, that provides information regarding shared resources. Some shared resources in the SoC have properties that are probed dynamically at boot by the remote processor. The information pertaining to the SoC and the platform are made available in the shared memory. Drivers can query this information using predefined strings. Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- *snip* diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c new file mode 100644 index ..050a56da76c8 --- /dev/null +++ b/drivers/soc/qcom/cmd-db.c @@ -0,0 +1,321 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#define NUM_PRIORITY 2 +#define MAX_SLV_ID 8 +#define CMD_DB_MAGIC 0x0C0330DBUL +#define SLAVE_ID_MASK 0x7 +#define SLAVE_ID_SHIFT 16 + +#define ENTRY_HEADER(hdr) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr->header_offset) + +#define RSC_OFFSET(hdr, ent) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr.data_offset + ent.offset) + +#define MIN(a, b) (((a) < (b)) ? (a) : (b)) I'm not sure if this was addressed before. Why use a custom macro and not min() or min_t()? I didn't realize the existance of this. cscope brought out a ton of 'min' defn. Will fix in the next spin. Thanks, Lina
[PATCH v5 0/2] irqchip: qcom: add support for PDC interrupt controller
Changes since v4: - Code clean up as suggested by Marc On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ RFC v2: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1600634.html v3: https://lkml.org/lkml/2018/2/6/595 v4: https://www.spinics.net/lists/linux-arm-msm/msg32906.html Lina Iyer (2): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: descibe PDC device binding .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 312 + 4 files changed, 400 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v5 2/2] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..0b2c97ddb520 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,78 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + Must be 2. + The first element of the tuple is the PDC pin for the + interrupt. + The second element is the trigger type. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-ranges: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the starting PDC port. + The second element is the GIC hwirq number for the PDC port. + The third element is the number of interrupts in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, must do - + + wake-device { + interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>; + }; + +In this case interrupt 514 would be mapped to port 2 on the PDC as defined by +the qcom,pdc-ranges property. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v5 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 312 + 3 files changed, 322 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index c70476b34a53..506c6aa7f0b4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -343,4 +343,13 @@ config MESON_IRQ_GPIO help Support Meson SoC Family GPIO Interrupt Multiplexer +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d2df34a54d38..280723d83916 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..c8b535d1b2b1 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,312 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Coyright (c) 2017-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_region { + u32 pin_base; + u32 parent_base; + u32 cnt; +}; + +static DEFINE_RAW_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; +static struct pdc_pin_region *pdc_region; +static int pdc_region_cnt; + +static void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + + index = pin_out / 32; + mask = pin_out % 32; + + raw_spin_lock(&pdc_lock); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + raw_spin_unlock(&pdc_lock); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level sensitive active High HIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, +}; + +/** + * qcom_pdc_g
[PATCH v6 0/2] irqchip: qcom: add support for PDC interrupt controller
Changes since v5: - Fixed comment style of SPDX Changes since v4: - Code clean up as suggested by Marc On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ RFC v2: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1600634.html v3: https://lkml.org/lkml/2018/2/6/595 v4: https://www.spinics.net/lists/linux-arm-msm/msg32906.html Lina Iyer (2): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: descibe PDC device binding .../bindings/interrupt-controller/qcom,pdc.txt | 78 + drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 314 + 4 files changed, 402 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v6 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 314 + 3 files changed, 324 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index c70476b34a53..506c6aa7f0b4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -343,4 +343,13 @@ config MESON_IRQ_GPIO help Support Meson SoC Family GPIO Interrupt Multiplexer +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d2df34a54d38..280723d83916 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..714e95858837 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,314 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_region { + u32 pin_base; + u32 parent_base; + u32 cnt; +}; + +static DEFINE_RAW_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; +static struct pdc_pin_region *pdc_region; +static int pdc_region_cnt; + +static void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + + index = pin_out / 32; + mask = pin_out % 32; + + raw_spin_lock(&pdc_lock); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + raw_spin_unlock(&pdc_lock); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level sensitive active High HIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, +}; + +/** + * qcom_
[PATCH v6 2/2] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..0b2c97ddb520 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,78 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + Must be 2. + The first element of the tuple is the PDC pin for the + interrupt. + The second element is the trigger type. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-ranges: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the starting PDC port. + The second element is the GIC hwirq number for the PDC port. + The third element is the number of interrupts in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, must do - + + wake-device { + interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>; + }; + +In this case interrupt 514 would be mapped to port 2 on the PDC as defined by +the qcom,pdc-ranges property. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH RFC v2 3/3] drivers: irqchip: pdc: log PDC info in FTRACE
From: Archana Sathyakumar Log key PDC pin configuration in FTRACE. Cc: Steven Rostedt Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/qcom-pdc.c | 7 ++ include/trace/events/pdc.h | 55 ++ 2 files changed, 62 insertions(+) create mode 100644 include/trace/events/pdc.h diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index a392380eada6..7f177ad88713 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -26,6 +26,8 @@ #include #include #include +#define CREATE_TRACE_POINTS +#include "trace/events/pdc.h" #define PDC_MAX_IRQS 126 @@ -68,6 +70,8 @@ static inline void pdc_enable_intr(struct irq_data *d, bool on) enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); pdc_reg_write(IRQ_ENABLE_BANK, index, enable); spin_unlock_irqrestore(&pdc_lock, flags); + + trace_irq_pin_config(PDC_ENTRY, pin_out, (u64)d->chip_data, 0, on); } static void qcom_pdc_gic_mask(struct irq_data *d) @@ -149,6 +153,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type); + trace_irq_pin_config(PDC_TYPE_CONFIG, pin_out, (u64)d->chip_data, + pdc_type, 0); + return irq_chip_set_type_parent(d, type); } diff --git a/include/trace/events/pdc.h b/include/trace/events/pdc.h new file mode 100644 index ..0e894bbc9e85 --- /dev/null +++ b/include/trace/events/pdc.h @@ -0,0 +1,55 @@ +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM pdc + +#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_PDC_H_ + +#include + +#define PDC_ENTRY 1 +#define PDC_TYPE_CONFIG2 + +TRACE_EVENT(irq_pin_config, + + TP_PROTO(u32 func, int pin, u64 hwirq, u32 type, u32 enable), + + TP_ARGS(func, pin, hwirq, type, enable), + + TP_STRUCT__entry( + __field(u32, func) + __field(int, pin) + __field(u64, hwirq) + __field(u32, type) + __field(u32, enable) + ), + + TP_fast_assign( + __entry->func = func; + __entry->pin = pin; + __entry->hwirq = hwirq; + __entry->type = type; + __entry->enable = enable; + ), + + TP_printk("%s hwirq:%lu pin:%d type:%u enable:%u", + print_symbolic(__entry->func, + { PDC_ENTRY, "pdc_entry" }, + { PDC_TYPE_CONFIG, "pdc_type_config" }), + __entry->pin, __entry->hwirq, __entry->type, __entry->enable) +); + +#endif +#define TRACE_INCLUDE_FILE pdc +#include -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH RFC v2 0/3] irqchip: qcom: add support for PDC interrupt controller
Changes in v2: - Drviers will specify PDC pin as their interrupt and PDC as interrupt parent - Updated driver to pick up PDC pin mappings from DT - Cleanup driver per Marc's suggestions - Addressed DT bindings comments from Rob H - Addressed FTRACE comments from Steven R On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ Lina Iyer (3): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: descibe PDC device binding drivers: irqchip: pdc: log PDC info in FTRACE .../bindings/interrupt-controller/qcom,pdc.txt | 78 + drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 333 + include/trace/events/pdc.h | 55 5 files changed, 476 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c create mode 100644 include/trace/events/pdc.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH RFC v2 2/3] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..7bf40cb6a4f8 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,78 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + The value must match that of the parent interrupt + controller defined in the DT. + The encoding of these cells are same as described in [1]. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-range: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the staring PDC port num. + The second element is the hwirq number for the PDC port. + The third element is the number of elements in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <3>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +The DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, would look like this - + + wake-device { + [...] + interrupt-parent = <&pdc>; + interrupt = <0 2 0>; + }; + +[1]. Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH RFC v2 1/3] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 326 + 3 files changed, 336 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index c70476b34a53..506c6aa7f0b4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -343,4 +343,13 @@ config MESON_IRQ_GPIO help Support Meson SoC Family GPIO Interrupt Multiplexer +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d2df34a54d38..280723d83916 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..a392380eada6 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,326 @@ +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_data { + int pin; + int hwirq; +}; + +static DEFINE_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; + +static inline void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static inline u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static inline void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + unsigned long flags; + + index = pin_out / 32; + mask = pin_out % 32; + + spin_lock_irqsave(&pdc_lock, flags); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + spin_unlock_irqrestore(&pdc_lock, flags); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level senstive active H
Re: [PATCH RFC v2 1/3] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Fri, Feb 02 2018 at 16:21 +, Marc Zyngier wrote: Hi Lina, On 02/02/18 14:21, Lina Iyer wrote: From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- +struct pdc_pin_data { + int pin; + int hwirq; +}; I seriously doubt you need this structure, see below. + +static DEFINE_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; You only have one of those per SoC? There is only one that Linux can control. +static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) +{ + int pin_out = d->hwirq; + enum pdc_irq_config_bits pdc_type; + + switch (type) { + case IRQ_TYPE_EDGE_RISING: + pdc_type = PDC_RISING_EDGE; + type = IRQ_TYPE_EDGE_RISING; + break; + case IRQ_TYPE_EDGE_FALLING: + pdc_type = PDC_FALLING_EDGE; + type = IRQ_TYPE_EDGE_RISING; + break; + case IRQ_TYPE_EDGE_BOTH: + pdc_type = PDC_DUAL_EDGE; + break; + case IRQ_TYPE_LEVEL_HIGH: + pdc_type = PDC_POLARITY_HIGH; + type = IRQ_TYPE_LEVEL_HIGH; + break; + case IRQ_TYPE_LEVEL_LOW: + pdc_type = PDC_POLARITY_LOW; + type = IRQ_TYPE_LEVEL_HIGH; + break; + default: + pdc_type = PDC_POLARITY_HIGH; + break; If this default clause triggers, something is pretty wrong. You may want to warn and bail out instead. The hardware defaults to this. I can bail out as well. + } + + pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type); + + return irq_chip_set_type_parent(d, type); +} + +static struct irq_chip qcom_pdc_gic_chip = { + .name = "pdc-gic", + .irq_eoi= irq_chip_eoi_parent, + .irq_mask = qcom_pdc_gic_mask, + .irq_unmask = qcom_pdc_gic_unmask, + .irq_retrigger = irq_chip_retrigger_hierarchy, + .irq_set_type = qcom_pdc_gic_set_type, + .flags = IRQCHIP_MASK_ON_SUSPEND | + IRQCHIP_SET_TYPE_MASKED | + IRQCHIP_SKIP_SET_WAKE, + .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent, +#ifdef CONFIG_SMP + .irq_set_affinity = irq_chip_set_affinity_parent, +#endif +}; + +static irq_hw_number_t get_irq_for_pin(int pin, struct pdc_pin_data *pdc_data) +{ + int i; + + for (i = 0; pdc_data[i].pin >= 0; i++) + if (pdc_data[i].pin == pin) + return pdc_data[i].hwirq; + + return pin; +} This is the function that irks me. The DT already gives you a nice set of ranges with all the information you need. And yet, you expand the whole thing into at most 127 structures, wasting memory and making the search time a function of the number of interrupts instead of being that of the number of regions. Not very nice. How about something like this (untested): Duh! Why didn't I think of this.. Thanks. struct pin_region { u32 pin_base; u32 parent_base; u32 cnt; }; struct pin_region *pin_regions; int pin_region_count; static int pdc_pin_to_parent_hwirq(int pin) { int i; for (i = 0; i < pin_region_count; i++) { if (pin >= pin_regions[i].pin_base && pin < (pin_regions[i].pin_base + pin_regions[i].cnt) return (pin_regions[i].parent_base + pin - pin_regions[i].pin_base); } WARN(); return -1; } Less memory, less comparisons. Agreed. + +static int qcom_pdc_translate(struct irq_domain *d, + struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type) +{ + if (is_of_node(fwspec->fwnode)) { + if (fwspec->param_count < 3) + return -EINVAL; Why 3? Reading the DT binding, this is indeed set to 3 without any reason. I'd suggest this becomes 2, encoding the pin number and the trigger information, as the leading 0 is quite useless. Yes, I know other examples in the kernel are using this 0, and that was a consequence of retrofitting the omitted interrupt controllers (back in the days of the stupid gic_arch_extn...
Re: [PATCH RFC v2 1/3] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
All valid comments. Will fix them all in the next rev. Thanks Thomas. -- Lina On Fri, Feb 02 2018 at 15:37 +, Thomas Gleixner wrote: On Fri, 2 Feb 2018, Lina Iyer wrote: +static inline void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + unsigned long flags; + + index = pin_out / 32; + mask = pin_out % 32; + + spin_lock_irqsave(&pdc_lock, flags); Please make this a raw spinlock. Aside of that the _irqsave() is pointless as the chip callbacks are already called with interrupts disabled. + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); You really should cache the enable register content to avoid the read back + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + spin_unlock_irqrestore(&pdc_lock, flags); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level senstive active HighHIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, // 0 00 What's wrong with PDC_POLARITY_LOW = 000b, PDC_FALLING_EDGE = 010b, instead of decimal and these weird comments ? +static irq_hw_number_t get_irq_for_pin(int pin, struct pdc_pin_data *pdc_data) +{ + int i; + + for (i = 0; pdc_data[i].pin >= 0; i++) + if (pdc_data[i].pin == pin) + return pdc_data[i].hwirq; Please let the for() loop have braces. See: https://marc.info/?l=linux-kernel&m=148467980905537&w=2 + + return pin; +} + +static int qcom_pdc_translate(struct irq_domain *d, + struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type) Please align the arguments right of the opening brace: static int qcom_pdc_translate(struct irq_domain *d, struct irq_fwspec *fwspec, unsigned long *hwirq, unsigned int *type) +{ + if (is_of_node(fwspec->fwnode)) { + if (fwspec->param_count < 3) + return -EINVAL; + + *hwirq = fwspec->param[1]; + *type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK; + return 0; + } + + return -EINVAL; +} + +static int qcom_pdc_alloc(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs, void *data) Ditto +static int pdc_setup_pin_mapping(struct device_node *np, + struct pdc_pin_data **data) +{ + int ret; + int n, i, j, k, pins = 0; + int *val; I have no idea what's the rationale behind these 3 lines of int declarations. + struct pdc_pin_data *map; + + n = of_property_count_elems_of_size(np, "qcom,pdc-ranges", sizeof(u32)); + if (!n || n % 3) + return -EINVAL; + + val = kcalloc(n, sizeof(u32), GFP_KERNEL); + if (!val) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "qcom,pdc-ranges", val, n); + if (ret) + return ret; + + for (i = 0; i < n; i += 3) + pins += val[i + 2]; + + if (pins > PDC_MAX_IRQS) + return -EFAULT; + + map = kcalloc(pins + 1, sizeof(*map), GFP_KERNEL); + if (!map) { + ret = -ENOMEM; + goto fail; + } + + for (i = 0, k = 0; i < n; i += 3) { + for (j = 0; j < val[i + 2]; j++, k++) { + map[k].pin = val[i] + j; + map[k].hwirq = val[i + 1] + j; + } + } This all is really horrible to read. First of all the val[] array. That can be represented in a structure, right? Without looking at the DT patch the code lets me assume: struct pdcrange { u32 pin; u32 hwirq; u32 numpins; u32
Re: [PATCH RFC v2 2/3] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
On Fri, Feb 02 2018 at 16:28 +, Marc Zyngier wrote: On 02/02/18 14:21, Lina Iyer wrote: From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 78 ++ 1 file changed, 78 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..7bf40cb6a4f8 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,78 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + The value must match that of the parent interrupt + controller defined in the DT. + The encoding of these cells are same as described in [1]. There shouldn't be such a requirement. These are two independent pieces of HW, and the first parameter doesn't mean anything for the PDC. Wouldn't that be confusing - that we have different definitions for interrupts in the same DT? I agree that they are different interrupt controllers, but it just feels odd. I will change this to just take 2 cells. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-range: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the staring PDC port num. + The second element is the hwirq number for the PDC port. + The third element is the number of elements in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <3>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +The DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, would look like this - + + wake-device { + [...] + interrupt-parent = <&pdc>; + interrupt = <0 2 0>; Again: 0 is not a valid trigger value. Ok. Thanks, Lina
Re: [PATCH RFC v2 3/3] drivers: irqchip: pdc: log PDC info in FTRACE
On Fri, Feb 02 2018 at 16:32 +, Steven Rostedt wrote: On Fri, 2 Feb 2018 07:22:00 -0700 Lina Iyer wrote: Hi Lina, This looks really good. I have one nit below. From: Archana Sathyakumar Log key PDC pin configuration in FTRACE. Cc: Steven Rostedt Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/qcom-pdc.c | 7 ++ include/trace/events/pdc.h | 55 ++ 2 files changed, 62 insertions(+) create mode 100644 include/trace/events/pdc.h diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index a392380eada6..7f177ad88713 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -26,6 +26,8 @@ #include #include #include +#define CREATE_TRACE_POINTS +#include "trace/events/pdc.h" #define PDC_MAX_IRQS 126 @@ -68,6 +70,8 @@ static inline void pdc_enable_intr(struct irq_data *d, bool on) enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); pdc_reg_write(IRQ_ENABLE_BANK, index, enable); spin_unlock_irqrestore(&pdc_lock, flags); + + trace_irq_pin_config(PDC_ENTRY, pin_out, (u64)d->chip_data, 0, on); } static void qcom_pdc_gic_mask(struct irq_data *d) @@ -149,6 +153,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type); + trace_irq_pin_config(PDC_TYPE_CONFIG, pin_out, (u64)d->chip_data, + pdc_type, 0); I wonder if it makes more sense to just pass 'd' into the trace events, and then do the dereference there. The reason is to try to get as much code out of the calling path as possible. Even though trace events use jump labels and have no conditional branches, the code to call the function is still within the code using the trace events. By passing in 'd' and doing the redirect in the trace event code, we remove the setting up of the redirect from the caller, and save some cache lines in the process. Makes sense. Will fix it. Thanks Steve. -- Lina
Re: [PATCH RFC v2 3/3] drivers: irqchip: pdc: log PDC info in FTRACE
On Fri, Feb 02 2018 at 15:57 +, Thomas Gleixner wrote: On Fri, 2 Feb 2018, Lina Iyer wrote: +++ b/include/trace/events/pdc.h @@ -0,0 +1,55 @@ +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. Can you please use the proper SPDX identifiers instead of the boiler plate? Same for the driver source file. Sure. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM pdc + +#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_PDC_H_ + +#include + +#define PDC_ENTRY 1 +#define PDC_TYPE_CONFIG2 + +TRACE_EVENT(irq_pin_config, This is really a too generic name for a PDC specific breakpoint. Hmm.. right. Aside of that the question is whether this really justifies a trace point. Wouldn't it be sufficient to use the GENERIC_IRQ_DEBUGFS infrastructure to make this accessible via debugfs? Some product configurations disable debugfs. I will ask around if this can be dropped. -- Lina
Re: [PATCH RFC v2 3/3] drivers: irqchip: pdc: log PDC info in FTRACE
On Fri, Feb 02 2018 at 23:02 +, Lina Iyer wrote: On Fri, Feb 02 2018 at 15:57 +, Thomas Gleixner wrote: On Fri, 2 Feb 2018, Lina Iyer wrote: +++ b/include/trace/events/pdc.h @@ -0,0 +1,55 @@ +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. Can you please use the proper SPDX identifiers instead of the boiler plate? Same for the driver source file. Sure. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM pdc + +#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_PDC_H_ + +#include + +#define PDC_ENTRY 1 +#define PDC_TYPE_CONFIG2 + +TRACE_EVENT(irq_pin_config, This is really a too generic name for a PDC specific breakpoint. Hmm.. right. Aside of that the question is whether this really justifies a trace point. Wouldn't it be sufficient to use the GENERIC_IRQ_DEBUGFS infrastructure to make this accessible via debugfs? Some product configurations disable debugfs. I will ask around if this can be dropped. Memory dumps after a crash have support for FTRACE and it helps greatly on production issues. Hence the preference for FTRACE support. -- Lina
Re: [PATCH RFC v2 3/3] drivers: irqchip: pdc: log PDC info in FTRACE
Hi Steve, On Fri, Feb 02 2018 at 14:22 +, Lina Iyer wrote: From: Archana Sathyakumar Log key PDC pin configuration in FTRACE. Cc: Steven Rostedt Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/qcom-pdc.c | 7 ++ include/trace/events/pdc.h | 55 ++ 2 files changed, 62 insertions(+) create mode 100644 include/trace/events/pdc.h diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index a392380eada6..7f177ad88713 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -26,6 +26,8 @@ #include #include #include +#define CREATE_TRACE_POINTS +#include "trace/events/pdc.h" In addition to this PDC patch, there are a few drivers with FTRACE support coming for up QCOM SoCs. Would it make sense to open up a new sub-folder for SoC specific FTRACE like say, trace/events/soc/ trace/events/soc/qcom/ What would be your recommendation? At the very least, I am thinking of renaming this file to trace/events/qcom-pdc.h. Thanks, Lina
[PATCH v3 2/2] dt-bindings: introduce Command DB for QCOM SoCs
From: Mahesh Sivasubramanian Command DB provides information on shared resources like clocks, regulators etc., probed at boot by the remote subsytem and made available in shared memory. Cc: devicet...@vger.kernel.org Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- .../devicetree/bindings/arm/msm/cmd-db.txt | 30 ++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt new file mode 100644 index ..039be54fe9c4 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt @@ -0,0 +1,30 @@ +Command DB +- + +Command DB is a database that provides a mapping between resource key and the +resource address for a system resource managed by a remote processor. The data +is stored in a shared memory region and is loaded by the remote processor. + +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for +controlling shared resources. Depending on the board configuration the shared +resource properties may change. These properties are dynamically probed by the +remote processor and made available in the shared memory. + +The devicetree representation of the command DB driver should be: + +PROPERTIES: +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,cmd-db" + +Example: + + reserved-memory { + [...] + qcom,cmd-db@c3f000c { + reg = <0x0 0xc3f000c 0x0 0x8>, + <0x0 0x85fe 0x0 0x2>; + compatible = "qcom,cmd-db"; + }; + }; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 0/2] drivers/qcom: add Command DB support
Changes in v3: - use min_t instead of MIN - add cmd db memory to reserved memory region - use devm_memremap These patches add support for reading a shared memory database in the newer QCOM SoCs called Command DB. With the new architecture on SDM845, shared resources like clocks, regulators etc., have dynamic properties. These properties may change based on external components, board configurations or available feature set. A remote processor detects these parameters and fills up the database with the resource and available state information. Platform drivers that need these shared resources will need to query this database to get the address and properties and vote for the state. The information in the database is static. The database is read-only memory location that is available for Linux. A pre-defined string is used as a key into an entry in the database. Generally, platform drivers query the database only at init to get the information they need. [v1]: https://www.spinics.net/lists/linux-arm-msm/msg32462.html [v2]: https://lkml.org/lkml/2018/2/8/588 Lina Iyer (2): drivers: qcom: add command DB driver dt-bindings: introduce Command DB for QCOM SoCs .../devicetree/bindings/arm/msm/cmd-db.txt | 30 ++ drivers/of/platform.c | 1 + drivers/soc/qcom/Kconfig | 9 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 319 + include/soc/qcom/cmd-db.h | 50 6 files changed, 410 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 1/2] drivers: qcom: add command DB driver
From: Mahesh Sivasubramanian Command DB is a simple database in the shared memory of QCOM SoCs, that provides information regarding shared resources. Some shared resources in the SoC have properties that are probed dynamically at boot by the remote processor. The information pertaining to the SoC and the platform are made available in the shared memory. Drivers can query this information using predefined strings. Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- drivers/of/platform.c | 1 + drivers/soc/qcom/Kconfig | 9 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 319 ++ include/soc/qcom/cmd-db.h | 50 5 files changed, 380 insertions(+) create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c00d81dfac0b..26fb43847f4b 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -494,6 +494,7 @@ EXPORT_SYMBOL_GPL(of_platform_default_populate); #ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, + { .compatible = "qcom,cmd-db" }, { .compatible = "ramoops" }, {} }; diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index e050eb83341d..b12868a2b92d 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -3,6 +3,15 @@ # menu "Qualcomm SoC drivers" +config QCOM_COMMAND_DB + bool "Qualcomm Command DB" + depends on (ARCH_QCOM && OF) || COMPILE_TEST + help + Command DB queries shared memory by key string for shared system + resources. Platform drivers that require to set state of a shared + resource on a RPM-hardened platform must use this database to get + SoC specific identifier and information for the shared resources. + config QCOM_GLINK_SSR tristate "Qualcomm Glink SSR driver" depends on RPMSG diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index dcebf2814e6d..bbd1230fc441 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o obj-$(CONFIG_QCOM_GSBI)+= qcom_gsbi.o obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c new file mode 100644 index ..0792a2a98fc9 --- /dev/null +++ b/drivers/soc/qcom/cmd-db.c @@ -0,0 +1,319 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#define NUM_PRIORITY 2 +#define MAX_SLV_ID 8 +#define CMD_DB_MAGIC 0x0C0330DBUL +#define SLAVE_ID_MASK 0x7 +#define SLAVE_ID_SHIFT 16 + +#define ENTRY_HEADER(hdr) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr->header_offset) + +#define RSC_OFFSET(hdr, ent) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr.data_offset + ent.offset) + +/** + * entry_header: header for each entry in cmddb + * + * @id: resource's identifier + * @priority: unused + * @addr: the address of the resource + * @len: length of the data + * @offset: offset at which data starts + */ +struct entry_header { + u64 id; + u32 priority[NUM_PRIORITY]; + u32 addr; + u16 len; + u16 offset; +}; + +/** + * rsc_hdr: resource header information + * + * @slv_id: id for the resource + * @header_offset: Entry header offset from data + * @data_offset: Entry offset for data location + * @cnt: number of entries for HW type + * @version: MSB is major, LSB is minor + */ +struct rsc_hdr { + u16 slv_id; + u16 header_offset; + u16 data_offset; + u16 cnt; + u16 version; + u16 reserved[3]; +}; + +/** + * cmd_db_header: The DB header information + * + * @version: The cmd db version + * @magic_number: constant expected in the database + * @header: array of resources + * @check_sum: check sum for the header. Unused. + * @reserved: reserved memory + * @data: driver specific data + */ +struct cmd_db_header { + u32 version; + u32 magic_num; + struct rsc_hdr header[MAX_SLV_ID]; + u32 check_sum; + u32 reserved; + u8 data[]; +}; + +/** + * DOC: Description of the Command DB database. + * + * At the start of the command DB memory is the cmd_db_header structure. + * The cmd_db_header holds the version, checksum, magic key as well as an + * array for header for each slave (depi
Re: [PATCH v2 01/10] drivers: qcom: rpmh-rsc: add RPMH controller for QCOM SoCs
Thanks Evan for your review. On Fri, Feb 16 2018 at 21:30 +, Evan Green wrote: Hi Lina, On Thu, Feb 15, 2018 at 9:34 AM, Lina Iyer wrote: + +/** + * tcs_response: Response object for a request Can you embed the acronym definition, ie: tcs_response: Responses for a Trigger Command Set. Sure. + * + * @drv: the controller + * @msg: the request for this response + * @m: the tcs identifier + * @err: error reported in the response + * @list: link list object. + */ +struct tcs_response { + struct rsc_drv *drv; + struct tcs_request *msg; + u32 m; + int err; + struct list_head list; +}; + +/** + * tcs_group: group of TCSes for a request state + * Document @drv. OK + * @type: type of the TCS in this group - active, sleep, wake + * @tcs_mask: mask of the TCSes relative to all the TCSes in the RSC + * @tcs_offset: start of the TCS group relative to the TCSes in the RSC + * @num_tcs: number of TCSes in this type + * @ncpt: number of commands in each TCS + * @tcs_lock: lock for synchronizing this TCS writes + * @responses: response objects for requests sent from each TCS + */ +struct tcs_group { + struct rsc_drv *drv; + int type; + u32 tcs_mask; + u32 tcs_offset; + int num_tcs; + int ncpt; + spinlock_t tcs_lock; + struct tcs_response *responses[MAX_TCS_PER_TYPE]; +}; + +/** + * rsc_drv: the RSC controller Would be more helpfully described as Resource State Coordinator controller. OK +static void write_tcs_reg_sync(struct rsc_drv *drv, int reg, int m, int n, + u32 data) +{ + write_tcs_reg(drv, reg, m, n, data); + for (;;) { + if (data == read_tcs_reg(drv, reg, m, n)) + break; + udelay(1); Should this time out and return a failure? There is no reason for this fail. We just need to ensure that it is written. Sometimes writes going through the bus, takes time to complete the write. When we exit this function, we are assured that it is written. + } +} + +static bool tcs_is_free(struct rsc_drv *drv, int m) +{ + return !atomic_read(&drv->tcs_in_use[m]) && + read_tcs_reg(drv, RSC_DRV_STATUS, m, 0); +} + +static struct tcs_group *get_tcs_of_type(struct rsc_drv *drv, int type) +{ + int i; + struct tcs_group *tcs; + + for (i = 0; i < TCS_TYPE_NR; i++) { + if (type == drv->tcs[i].type) + break; + } + + if (i == TCS_TYPE_NR) + return ERR_PTR(-EINVAL); + + tcs = &drv->tcs[i]; + if (!tcs->num_tcs) + return ERR_PTR(-EINVAL); + + return tcs; +} + +static struct tcs_group *get_tcs_for_msg(struct rsc_drv *drv, + struct tcs_request *msg) +{ + int type; + + switch (msg->state) { + case RPMH_ACTIVE_ONLY_STATE: + type = ACTIVE_TCS; + break; + default: + return ERR_PTR(-EINVAL); + } + + return get_tcs_of_type(drv, type); +} + +static void send_tcs_response(struct tcs_response *resp) +{ + struct rsc_drv *drv = resp->drv; + unsigned long flags; + + if (!resp) + return; Does this ever happen? Ah, I see that it might in the irq handler. But get_response already assumes that there is a response, and reaches through it. So I don't think you need this here nor the check+label in the irq handler. Is requesting an index out of range purely a developer error, or could it happen in some sort of runtime scarcity situation? If it's a developer error, I'd get rid of all the null checking. If it's something that might really happen under the right circumstances, then my comment above doesn't stand and you'd want to fix the null dereference in get_response instead. I added the check so that I dont have confusing goto in the IRQ handler. + + spin_lock_irqsave(&drv->drv_lock, flags); + INIT_LIST_HEAD(&resp->list); + list_add_tail(&resp->list, &drv->response_pending); + spin_unlock_irqrestore(&drv->drv_lock, flags); + + tasklet_schedule(&drv->tasklet); +} + +/** + * tcs_irq_handler: TX Done interrupt handler + */ +static irqreturn_t tcs_irq_handler(int irq, void *p) +{ + struct rsc_drv *drv = p; + int m, i; + u32 irq_status, sts; + struct tcs_response *resp; + struct tcs_cmd *cmd; + int err; + + irq_status = read_tcs_reg(drv, RSC_DRV_IRQ_STATUS, 0, 0); + + for (m = 0; m < drv->num_tcs; m++) { + if (!(irq_status & (u32)BIT(m))) + continue; + + err = 0; + resp = get_response(drv, m); + if (!resp) { I mention this above, but I don't think get_response can gracefully return null, so is this needed?
Re: [PATCH v6 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Mon, Feb 12 2018 at 13:40 +, Thomas Gleixner wrote: On Fri, 9 Feb 2018, Lina Iyer wrote: +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level sensitive active High HIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, My previous comment about using binary constants still stands. Please either address review comments or reply at least. Ignoring reviews is not an option. I removed them from the enum definitions. Will remove them from the comments as well. Sorry. It was not my intention to ignore any review comments. Aside of that I really have to ask about the naming of these constants. Are these names hardware register nomenclature? If yes, they are disgusting. If no, they are still disgusting, but should be changed to sensible ones, which just match the IRQ_TYPE naming convention. PDC_LEVEL_LOW= 000b, PDC_EDGE_FALLING = 010b, They are named that way in spec :) Will change. + switch (type) { + case IRQ_TYPE_EDGE_RISING: + pdc_type = PDC_RISING_EDGE; + type = IRQ_TYPE_EDGE_RISING; Whats the point of assigning the same value again? Failed to notice. Will fix. Thanks, Lina + break; + case IRQ_TYPE_EDGE_FALLING: + pdc_type = PDC_FALLING_EDGE; + type = IRQ_TYPE_EDGE_RISING; + break; + case IRQ_TYPE_EDGE_BOTH: + pdc_type = PDC_DUAL_EDGE; + break; + case IRQ_TYPE_LEVEL_HIGH: + pdc_type = PDC_POLARITY_HIGH; + type = IRQ_TYPE_LEVEL_HIGH; Ditto Thanks, tglx
[PATCH v4 2/2] dt-bindings: introduce Command DB for QCOM SoCs
From: Mahesh Sivasubramanian Command DB provides information on shared resources like clocks, regulators etc., probed at boot by the remote subsytem and made available in shared memory. Cc: devicet...@vger.kernel.org Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- Changes in v4: - Fix unwanted capitalization - Add reg property --- .../devicetree/bindings/arm/msm/cmd-db.txt | 38 ++ 1 file changed, 38 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt diff --git a/Documentation/devicetree/bindings/arm/msm/cmd-db.txt b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt new file mode 100644 index ..5737ed2ac6e8 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/msm/cmd-db.txt @@ -0,0 +1,38 @@ +Command DB +- + +Command DB is a database that provides a mapping between resource key and the +resource address for a system resource managed by a remote processor. The data +is stored in a shared memory region and is loaded by the remote processor. + +Some of the Qualcomm Technologies Inc SoC's have hardware accelerators for +controlling shared resources. Depending on the board configuration the shared +resource properties may change. These properties are dynamically probed by the +remote processor and made available in the shared memory. + +The bindings for Command DB is specified in the reserved-memory section in +devicetree. The devicetree representation of the command DB driver should be: + +Properties: +- compatible: + Usage: required + Value type: + Definition: Should be "qcom,cmd-db" + +- reg: + Usage: required + Value type: + Definition: The register address that points to the location of the + Command DB in memory. Additionally, specify the address + and size of the actual lacation in memory. + +Example: + + reserved-memory { + [...] + qcom,cmd-db@c3f000c { + reg = <0x0 0xc3f000c 0x0 0x8>, + <0x0 0x85fe 0x0 0x2>; + compatible = "qcom,cmd-db"; + }; + }; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v4 0/2] drivers/qcom: add Command DB support
Changes in v3: - DT binding documentation fixes as suggested by Rob. These patches add support for reading a shared memory database in the newer QCOM SoCs called Command DB. With the new architecture on SDM845, shared resources like clocks, regulators etc., have dynamic properties. These properties may change based on external components, board configurations or available feature set. A remote processor detects these parameters and fills up the database with the resource and available state information. Platform drivers that need these shared resources will need to query this database to get the address and properties and vote for the state. The information in the database is static. The database is read-only memory location that is available for Linux. A pre-defined string is used as a key into an entry in the database. Generally, platform drivers query the database only at init to get the information they need. [v1]: https://www.spinics.net/lists/linux-arm-msm/msg32462.html [v2]: https://lkml.org/lkml/2018/2/8/588 [v3]: https://lkml.org/lkml/2018/2/16/842 Lina Iyer (2): drivers: qcom: add command DB driver dt-bindings: introduce Command DB for QCOM SoCs .../devicetree/bindings/arm/msm/cmd-db.txt | 38 +++ drivers/of/platform.c | 1 + drivers/soc/qcom/Kconfig | 9 + drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 319 + include/soc/qcom/cmd-db.h | 50 6 files changed, 418 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/msm/cmd-db.txt create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v4 1/2] drivers: qcom: add command DB driver
From: Mahesh Sivasubramanian Command DB is a simple database in the shared memory of QCOM SoCs, that provides information regarding shared resources. Some shared resources in the SoC have properties that are probed dynamically at boot by the remote processor. The information pertaining to the SoC and the platform are made available in the shared memory. Drivers can query this information using predefined strings. Signed-off-by: Mahesh Sivasubramanian Signed-off-by: Lina Iyer --- drivers/of/platform.c | 1 + drivers/soc/qcom/Kconfig | 9 ++ drivers/soc/qcom/Makefile | 1 + drivers/soc/qcom/cmd-db.c | 319 ++ include/soc/qcom/cmd-db.h | 50 5 files changed, 380 insertions(+) create mode 100644 drivers/soc/qcom/cmd-db.c create mode 100644 include/soc/qcom/cmd-db.h diff --git a/drivers/of/platform.c b/drivers/of/platform.c index c00d81dfac0b..26fb43847f4b 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -494,6 +494,7 @@ EXPORT_SYMBOL_GPL(of_platform_default_populate); #ifndef CONFIG_PPC static const struct of_device_id reserved_mem_matches[] = { { .compatible = "qcom,rmtfs-mem" }, + { .compatible = "qcom,cmd-db" }, { .compatible = "ramoops" }, {} }; diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig index e050eb83341d..b12868a2b92d 100644 --- a/drivers/soc/qcom/Kconfig +++ b/drivers/soc/qcom/Kconfig @@ -3,6 +3,15 @@ # menu "Qualcomm SoC drivers" +config QCOM_COMMAND_DB + bool "Qualcomm Command DB" + depends on (ARCH_QCOM && OF) || COMPILE_TEST + help + Command DB queries shared memory by key string for shared system + resources. Platform drivers that require to set state of a shared + resource on a RPM-hardened platform must use this database to get + SoC specific identifier and information for the shared resources. + config QCOM_GLINK_SSR tristate "Qualcomm Glink SSR driver" depends on RPMSG diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile index dcebf2814e6d..bbd1230fc441 100644 --- a/drivers/soc/qcom/Makefile +++ b/drivers/soc/qcom/Makefile @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o obj-$(CONFIG_QCOM_GLINK_SSR) +=glink_ssr.o obj-$(CONFIG_QCOM_GSBI)+= qcom_gsbi.o obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c new file mode 100644 index ..0792a2a98fc9 --- /dev/null +++ b/drivers/soc/qcom/cmd-db.c @@ -0,0 +1,319 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#define NUM_PRIORITY 2 +#define MAX_SLV_ID 8 +#define CMD_DB_MAGIC 0x0C0330DBUL +#define SLAVE_ID_MASK 0x7 +#define SLAVE_ID_SHIFT 16 + +#define ENTRY_HEADER(hdr) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr->header_offset) + +#define RSC_OFFSET(hdr, ent) ((void *)cmd_db_header +\ + sizeof(*cmd_db_header) +\ + hdr.data_offset + ent.offset) + +/** + * entry_header: header for each entry in cmddb + * + * @id: resource's identifier + * @priority: unused + * @addr: the address of the resource + * @len: length of the data + * @offset: offset at which data starts + */ +struct entry_header { + u64 id; + u32 priority[NUM_PRIORITY]; + u32 addr; + u16 len; + u16 offset; +}; + +/** + * rsc_hdr: resource header information + * + * @slv_id: id for the resource + * @header_offset: Entry header offset from data + * @data_offset: Entry offset for data location + * @cnt: number of entries for HW type + * @version: MSB is major, LSB is minor + */ +struct rsc_hdr { + u16 slv_id; + u16 header_offset; + u16 data_offset; + u16 cnt; + u16 version; + u16 reserved[3]; +}; + +/** + * cmd_db_header: The DB header information + * + * @version: The cmd db version + * @magic_number: constant expected in the database + * @header: array of resources + * @check_sum: check sum for the header. Unused. + * @reserved: reserved memory + * @data: driver specific data + */ +struct cmd_db_header { + u32 version; + u32 magic_num; + struct rsc_hdr header[MAX_SLV_ID]; + u32 check_sum; + u32 reserved; + u8 data[]; +}; + +/** + * DOC: Description of the Command DB database. + * + * At the start of the command DB memory is the cmd_db_header structure. + * The cmd_db_header holds the version, checksum, magic key as well as an + * array for header for each slave (depi
[PATCH v3 0/2] irqchip: qcom: add support for PDC interrupt controller
Changes since RFC v2: - Fixed up DT probe based on suggestions from Thomas and Marc - Code clean up as suggested by Thomas - Switch to SPDX license marker - Dropped the FTRACE patch for now, will need more thought and discussions On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ RFC v2: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1600634.html Lina Iyer (2): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: descibe PDC device binding .../bindings/interrupt-controller/qcom,pdc.txt | 80 ++ drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 302 + 4 files changed, 392 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 2/2] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 80 ++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..1f26e4853e7f --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,80 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + Must be 2. + The first element of the tuple is the PDC pin for the + interrupt. + The second element is the trigger type. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-ranges: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the staring PDC port num. + The second element is the hwirq number for the PDC port. + The third element is the number of interrupts in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, must do - + + wake-device { + [...] + interrupt-controller = <&pdc>; + interrupt = <2 IRQ_TYPE_LEVEL_HIGH>; + }; + +In this case interrupt 514 would be mapped to port 2 on the PDC as defined by +the qcom,pdc-ranges property. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v3 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 302 + 3 files changed, 312 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index c70476b34a53..506c6aa7f0b4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -343,4 +343,13 @@ config MESON_IRQ_GPIO help Support Meson SoC Family GPIO Interrupt Multiplexer +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d2df34a54d38..280723d83916 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..88fa650f0653 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,302 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Coyright (c) 2017-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_region { + u32 pin_base; + u32 parent_base; + u32 cnt; +}; + +static DEFINE_RAW_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; +static struct pdc_pin_region *pdc_region; +static int pdc_region_cnt; + +static inline void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static inline u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static inline void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + + index = pin_out / 32; + mask = pin_out % 32; + + raw_spin_lock(&pdc_lock); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + raw_spin_unlock(&pdc_lock); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level senstive active HighHIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, +}; +
Re: [PATCH v3 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Tue, Feb 06 2018 at 20:34 +, Marc Zyngier wrote: On Tue, 06 Feb 2018 18:09:04 +, Lina Iyer wrote: +#define PDC_MAX_IRQS 126 From v2: "Is that an absolute, architectural maximum? Or should it come from the DT (being the sum of all ranges that are provided by this PDC)?" Architectural maximum. Sorry I forgot to respond earlier. +static inline void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static inline u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static inline void pdc_enable_intr(struct irq_data *d, bool on) You can loose the "inline" on these 3 function, I believe the compiler will do a pretty good job specialising them. Ok. + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level senstive active HighHIGH sensitive Ok + +static struct irq_chip qcom_pdc_gic_chip = { const? + .name = "pdc-gic", Just call it "PDC". OK to both. + .irq_eoi= irq_chip_eoi_parent, + .irq_mask = qcom_pdc_gic_mask, + .irq_unmask = qcom_pdc_gic_unmask, + .irq_retrigger = irq_chip_retrigger_hierarchy, + .irq_set_type = qcom_pdc_gic_set_type, + .flags = IRQCHIP_MASK_ON_SUSPEND | + IRQCHIP_SET_TYPE_MASKED | + IRQCHIP_SKIP_SET_WAKE, + .irq_set_vcpu_affinity = irq_chip_set_vcpu_affinity_parent, +#ifdef CONFIG_SMP + .irq_set_affinity = irq_chip_set_affinity_parent, +#endif Is this supposed to work on any architecture other than arm64? If not, then you can loose the CONFIG_SMP, as there is no !SMP config on arm64. Only ARM64 afaict. But who knows ? :) +}; + +static irq_hw_number_t get_parent_hwirq(int pin) +{ + int i; + struct pdc_pin_region *region; + + for (i = 0; i < pdc_region_cnt; i++) { + region = &pdc_region[i]; + if (pin >= region->pin_base && + pin < region->pin_base + region->cnt) + return (region->parent_base + pin - region->pin_base); + } + + WARN_ON(1); + return pin; Do not return a valid value in case of error. Please return an error value that you will check in the caller. Otherwise you're feeding the GIC with a SPI number that is actually a PDC pin number. That is not going to have any positive effect. How about the max of irq_hw_number_t ? +static int qcom_pdc_alloc(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs, void *data) +{ + struct irq_fwspec *fwspec = data; + struct irq_fwspec parent_fwspec; + irq_hw_number_t hwirq, parent_hwirq; + unsigned int type; + int ret; + + ret = qcom_pdc_translate(domain, fwspec, &hwirq, &type); + if (ret) + return -EINVAL; + + parent_hwirq = get_parent_hwirq(hwirq); + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, + &qcom_pdc_gic_chip, + (void *)parent_hwirq); Nothing else in this driver should be concerned with the parent hwirq, so NULL should be an appropriate value for chip_data. Yes, I forgot to remove it. I don't need this anymore. + region = kcalloc(n, sizeof(*region), GFP_KERNEL); + if (!region) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "qcom,pdc-ranges", region, n); + if (ret) { + kfree(region); + return -EINVAL; + } + + pdc_region = (struct pdc_pin_region *)region; Oh please... Don't resort to that kind of hack. Next thing you know, someone will add a u8 field to that structure and you'll spend the following 2 hours trying to understand why it all went so wrong. Allocate a bounce buffer if you want, copy fields one by one, I don't care. Just don't do that. :-( :) ok. + pdc_region_cnt = n / 3; + + return 0; +} + +int qcom_pdc_init(struct device_node *node, struct device_node *parent) static. OK. +{ + struct irq_domain *parent_domain, *pdc_domain; + int ret; + + pdc_base = of_iomap(node, 0); + if (!pdc_base) { + pr_err("%s: unable to map PDC registers\n", node->full_name); + return -ENXIO; + } + + parent_domain = irq_find_host(parent); + if (!parent_domain) { + pr_err("%s: unable to obtain PDC parent domain\n", +
[PATCH v4 2/2] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
From: Archana Sathyakumar Add device binding documentation for the PDC Interrupt controller on QCOM SoC's like the SDM845. The interrupt-controller can be used to sense edge low interrupts and wakeup interrupts when the GIC is non-operational. Cc: devicet...@vger.kernel.org Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- .../bindings/interrupt-controller/qcom,pdc.txt | 80 ++ 1 file changed, 80 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt new file mode 100644 index ..21c4f2fbcd0b --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt @@ -0,0 +1,80 @@ +PDC interrupt controller + +Qualcomm Technologies Inc. SoCs based on the RPM Hardened architecture have a +Power Domain Controller (PDC) that is on always-on domain. In addition to +providing power control for the power domains, the hardware also has an +interrupt controller that can be used to help detect edge low interrupts as +well detect interrupts when the GIC is non-operational. + +GIC is parent interrupt controller at the highest level. Platform interrupt +controller PDC is next in hierarchy, followed by others. Drivers requiring +wakeup capabilities of their device interrupts routed through the PDC, must +specify PDC as their interrupt controller and request the PDC port associated +with the GIC interrupt. See example below. + +Properties: + +- compatible: + Usage: required + Value type: + Definition: Should contain "qcom,-pdc" + - "qcom,sdm845-pdc": For SDM845 + +- reg: + Usage: required + Value type: + Definition: Specifies the base physical address for PDC hardware. + +- interrupt-cells: + Usage: required + Value type: + Definition: Specifies the number of cells needed to encode an interrupt + source. + Must be 2. + The first element of the tuple is the PDC pin for the + interrupt. + The second element is the trigger type. + +- interrupt-parent: + Usage: required + Value type: + Definition: Specifies the interrupt parent necessary for hierarchical + domain to operate. + +- interrupt-controller: + Usage: required + Value type: + Definition: Identifies the node as an interrupt controller. + +- qcom,pdc-ranges: + Usage: required + Value type: + Definition: Specifies the PDC pin offset and the number of PDC ports. + The tuples indicates the valid mapping of valid PDC ports + and their hwirq mapping. + The first element of the tuple is the starting PDC port. + The second element is the GIC hwirq number for the PDC port. + The third element is the number of interrupts in sequence. + +Example: + + pdc: interrupt-controller@b22 { + compatible = "qcom,sdm845-pdc"; + reg = <0xb22 0x3>; + qcom,pdc-ranges = <0 512 94>, <94 641 15>, <115 662 7>; + #interrupt-cells = <2>; + interrupt-parent = <&intc>; + interrupt-controller; + }; + +DT binding of a device that wants to use the GIC SPI 514 as a wakeup +interrupt, must do - + + wake-device { + [...] + interrupt-controller = <&pdc>; + interrupt = <2 IRQ_TYPE_LEVEL_HIGH>; + }; + +In this case interrupt 514 would be mapped to port 2 on the PDC as defined by +the qcom,pdc-ranges property. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v4 0/2] irqchip: qcom: add support for PDC interrupt controller
Changes since v3: - Code clean up as suggested by Marc On newer Qualcomm Techonologies Inc's SoCs like the SDM845, the GIC is in a power domain that can be powered off when not needed. Interrupts that need to be sensed even when the GIC is powered off, are routed through an interrupt controller in an always-on domain called the Power Domain Controller a.k.a PDC. This series adds support for the PDC's interrupt controller. Please consider reviewing these patches. RFC v1: https://patchwork.kernel.org/patch/10180857/ RFC v2: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1600634.html v3: https://lkml.org/lkml/2018/2/6/595 Lina Iyer (2): drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs dt-bindings/interrupt-controller: pdc: descibe PDC device binding .../bindings/interrupt-controller/qcom,pdc.txt | 80 ++ drivers/irqchip/Kconfig| 9 + drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 311 + 4 files changed, 401 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt create mode 100644 drivers/irqchip/qcom-pdc.c -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v4 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
>From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- drivers/irqchip/Kconfig| 9 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/qcom-pdc.c | 311 + 3 files changed, 321 insertions(+) create mode 100644 drivers/irqchip/qcom-pdc.c diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index c70476b34a53..506c6aa7f0b4 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -343,4 +343,13 @@ config MESON_IRQ_GPIO help Support Meson SoC Family GPIO Interrupt Multiplexer +config QCOM_PDC + bool "QCOM PDC" + depends on ARCH_QCOM + select IRQ_DOMAIN + select IRQ_DOMAIN_HIERARCHY + help + Power Domain Controller driver to manage and configure wakeup + IRQs for Qualcomm Technologies Inc (QTI) mobile chips. + endmenu diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index d2df34a54d38..280723d83916 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,3 +84,4 @@ obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o obj-$(CONFIG_IRQ_UNIPHIER_AIDET) += irq-uniphier-aidet.o obj-$(CONFIG_ARCH_SYNQUACER) += irq-sni-exiu.o obj-$(CONFIG_MESON_IRQ_GPIO) += irq-meson-gpio.o +obj-$(CONFIG_QCOM_PDC) += qcom-pdc.o diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c new file mode 100644 index ..376e66c47868 --- /dev/null +++ b/drivers/irqchip/qcom-pdc.c @@ -0,0 +1,311 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Coyright (c) 2017-2018, The Linux Foundation. All rights reserved. */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define PDC_MAX_IRQS 126 + +#define CLEAR_INTR(reg, intr) (reg & ~(1 << intr)) +#define ENABLE_INTR(reg, intr) (reg | (1 << intr)) + +#define IRQ_ENABLE_BANK0x10 +#define IRQ_i_CFG 0x110 + +struct pdc_pin_region { + u32 pin_base; + u32 parent_base; + u32 cnt; +}; + +static DEFINE_RAW_SPINLOCK(pdc_lock); +static void __iomem *pdc_base; +static struct pdc_pin_region *pdc_region; +static int pdc_region_cnt; + +static void pdc_reg_write(int reg, u32 i, u32 val) +{ + writel_relaxed(val, pdc_base + reg + i * sizeof(u32)); +} + +static u32 pdc_reg_read(int reg, u32 i) +{ + return readl_relaxed(pdc_base + reg + i * sizeof(u32)); +} + +static void pdc_enable_intr(struct irq_data *d, bool on) +{ + int pin_out = d->hwirq; + u32 index, mask; + u32 enable; + + index = pin_out / 32; + mask = pin_out % 32; + + raw_spin_lock(&pdc_lock); + enable = pdc_reg_read(IRQ_ENABLE_BANK, index); + enable = on ? ENABLE_INTR(enable, mask) : CLEAR_INTR(enable, mask); + pdc_reg_write(IRQ_ENABLE_BANK, index, enable); + raw_spin_unlock(&pdc_lock); +} + +static void qcom_pdc_gic_mask(struct irq_data *d) +{ + pdc_enable_intr(d, false); + irq_chip_mask_parent(d); +} + +static void qcom_pdc_gic_unmask(struct irq_data *d) +{ + pdc_enable_intr(d, true); + irq_chip_unmask_parent(d); +} + +/* + * GIC does not handle falling edge or active low. To allow falling edge and + * active low interrupts to be handled at GIC, PDC has an inverter that inverts + * falling edge into a rising edge and active low into an active high. + * For the inverter to work, the polarity bit in the IRQ_CONFIG register has to + * set as per the table below. + * (polarity, falling edge, rising edge ) POLARITY + * 3'b0 00 Level sensitive active lowLOW + * 3'b0 01 Rising edge sensitive NOT USED + * 3'b0 10 Falling edge sensitiveLOW + * 3'b0 11 Dual Edge sensitive NOT USED + * 3'b1 00 Level sensitive active High HIGH + * 3'b1 01 Falling Edge sensitiveNOT USED + * 3'b1 10 Rising edge sensitive HIGH + * 3'b1 11 Dual Edge sensitive HIGH + */ +enum pdc_irq_config_bits { + PDC_POLARITY_LOW= 0, + PDC_FALLING_EDGE= 2, + PDC_POLARITY_HIGH = 4, + PDC_RISING_EDGE = 6, + PDC_DUAL_EDGE = 7, +}; + +/** + * qcom_pdc_g
Re: [PATCH v4 2/2] dt-bindings/interrupt-controller: pdc: descibe PDC device binding
On Wed, Feb 07 2018 at 16:43 +, Bjorn Andersson wrote: On Wed 07 Feb 07:49 PST 2018, Lina Iyer wrote: diff --git a/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt b/Documentation/devicetree/bindings/interrupt-controller/qcom,pdc.txt [..] +Example: [..] + wake-device { + [...] + interrupt-controller = <&pdc>; Sorry for not seeing this earlier, but this should be: interrupt-parent = <&pdc>; Thats right. Thanks for pointing out. Or as it's not unlikely that clients might use a mix of pdc and non-pdc interrupts the example could use the form: interrupts-extended = <&pdc 2 IRQ_TYPE_LEVEL_HIGH>; OK. Will add it as another example. + interrupt = <2 IRQ_TYPE_LEVEL_HIGH>; + }; Thanks, Lina
Re: [PATCH v4 1/2] drivers: irqchip: pdc: Add PDC interrupt controller for QCOM SoCs
On Wed, Feb 07 2018 at 16:43 +, Marc Zyngier wrote: On 07/02/18 15:49, Lina Iyer wrote: From : Archana Sathyakumar The Power Domain Controller (PDC) on QTI SoCs like SDM845 houses an interrupt controller along with other domain control functions to handle interrupt related functions like handle falling edge or active low which are not detected at the GIC and handle wakeup interrupts. The interrupt controller is on an always-on domain for the purpose of waking up the processor. Only a subset of the processor's interrupts are routed through the PDC to the GIC. The PDC powers on the processors' domain, when in low power mode and replays pending interrupts so the GIC may wake up the processor. Signed-off-by: Archana Sathyakumar Signed-off-by: Lina Iyer --- + parent_hwirq = get_parent_hwirq(hwirq); Now that you return a well known value on error, how about testing it and erroring out instead of propagating it to the parent? Hmm.. Okay. + region = kcalloc(n, sizeof(*region), GFP_KERNEL); + if (!region) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "qcom,pdc-ranges", region, n); + if (ret) + goto fail; + + pdc_region_cnt = n / 3; + pdc_region = kcalloc(pdc_region_cnt, sizeof(*pdc_region), GFP_KERNEL); + if (!pdc_region) { + pdc_region_cnt = 0; + ret = -ENOMEM; + goto fail; + } + + for (n = 0; n < pdc_region_cnt; n++, region += 3) { + pdc_region[n].pin_base= region[0]; + pdc_region[n].parent_base = region[1]; + pdc_region[n].cnt = region[2]; Here's an alternative version that doesn't require any bounce buffer: for (n = 0; n < pdc_region_cnt; n++) { ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 0, &pdc_region[n].pin_base); if (ret) goto fail; ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 1, &pdc_region[n].parent_base); if (ret) goto fail; ret = of_property_read_u32_index(np, "qcom,pdc-ranges", n * 3 + 2, &pdc_region[n].cnt); if (ret) goto fail; } And you can get rid of "region" altogether, because... + } + +fail: + kfree(region); ... now that once you've incremented "region" in your for() loop, this kfree isn't going to do what you think it does. Sure. + return ret; +} + +static int qcom_pdc_init(struct device_node *node, struct device_node *parent) +{ + struct irq_domain *parent_domain, *pdc_domain; + int ret; + + pdc_base = of_iomap(node, 0); + if (!pdc_base) { + pr_err("%pOF: unable to map PDC registers\n", node->full_name); The whole point of the %pOF specifier is that you don't pass the full_name field, but just the node itself. This is why I pointed you to commit ce4fecf1fe15 so that you could see how it is used... Oops. I had seen the commit earlier, hence I didn't pay close attention this time. Sorry, my bad. Thanks, Lina
Re: [PATCH v2 00/17] Make rpmsg a framework
Hi Bjorn, On Thu, Sep 01 2016 at 16:28 -0600, Bjorn Andersson wrote: This series splits the virtio rpmsg bus driver into a rpmsg bus and a virtio backend/wireformat. As we discussed the Qualcomm SMD implementation a couple of years back people suggested that I should make it "a rpmsg thingie". With the introduction of the Qualcomm 8996 platform, we must support a variant of the communication mechanism that share many of the characteristics of SMD, but are different enough that it can't be done in a single implementation. As such there is enough benefit to do the necessary work and being able to make SMD a "rpmsg thingie". On-top of this series I have patches to switch the current smd clients over to rpmsg (and by that drop the existing SMD implementation). All this allows me to implement the new backend and reuse all existing SMD drivers with the new mechanism. RPM Communication has to supported even when IRQs are disabled. The most important use of this communication is to set the wake up time for the CPU subsystem when all the CPUs are powered off. In addition to that, "sleep" votes that are sent by the application processor subsystem to allow system to go into deep sleep modes can only be triggered when the CPU PM domains are power collapsed, drivers do not have a knowledge of when that happens. This has to be done by a platform code that registers for CPU PM domain power_off/on callbacks. Using rpmsg may be nice for RPM SMD communication, but mutexes need to go away for this driver to be any useful than bare bones active mode resource requests for QCOM SoCs. By not doing that now, we lock ourselves out of using this SMD driver in the near future when CPU PM domains are available in the kernel with an ability to do system low power modes. I hope you would make rpmsg work in IRQ disabled contexts first before porting the SMD driver. Thanks, Lina Changes from v1: - Split up the patch moving core code to rpmsg_core into several commits - Dropped the wrapping struct in rpmsg_core and just added the ops to the public API (but hid the implementation details) - Reordered things to reduce the size of the later patches Bjorn Andersson (17): rpmsg: Enable matching devices with drivers based on DT rpmsg: Name rpmsg devices based on channel id rpmsg: rpmsg_send() operations takes rpmsg_endpoint rpmsg: Make rpmsg_create_ept() take channel_info struct rpmsg: Clean up rpmsg device vs channel naming rpmsg: Introduce indirection table for rpmsg_device operations rpmsg: Move rpmsg_device API to new file rpmsg: Indirection table for rpmsg_endpoint operations rpmsg: Move endpoint related interface to rpmsg core rpmsg: Move helper for finding rpmsg devices to core rpmsg: Split off generic tail of create_channel() rpmsg: Split rpmsg core and virtio backend rpmsg: Hide rpmsg indirection tables rpmsg: virtio: Hide vrp pointer from the public API rpmsg: Move virtio specifics from public header rpmsg: Allow callback to return errors rpmsg: Introduce Qualcomm SMD backend drivers/remoteproc/Kconfig |4 +- drivers/rpmsg/Kconfig | 14 + drivers/rpmsg/Makefile |4 +- drivers/rpmsg/qcom_smd.c| 1434 +++ drivers/rpmsg/rpmsg_core.c | 498 drivers/rpmsg/rpmsg_internal.h | 82 ++ drivers/rpmsg/virtio_rpmsg_bus.c| 487 +--- include/linux/rpmsg.h | 246 +- samples/rpmsg/rpmsg_client_sample.c | 14 +- 9 files changed, 2266 insertions(+), 517 deletions(-) create mode 100644 drivers/rpmsg/qcom_smd.c create mode 100644 drivers/rpmsg/rpmsg_core.c create mode 100644 drivers/rpmsg/rpmsg_internal.h -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/17] Make rpmsg a framework
On Mon, Sep 12 2016 at 10:52 -0600, Lina Iyer wrote: Hi Bjorn, On Thu, Sep 01 2016 at 16:28 -0600, Bjorn Andersson wrote: This series splits the virtio rpmsg bus driver into a rpmsg bus and a virtio backend/wireformat. As we discussed the Qualcomm SMD implementation a couple of years back people suggested that I should make it "a rpmsg thingie". With the introduction of the Qualcomm 8996 platform, we must support a variant of the communication mechanism that share many of the characteristics of SMD, but are different enough that it can't be done in a single implementation. As such there is enough benefit to do the necessary work and being able to make SMD a "rpmsg thingie". On-top of this series I have patches to switch the current smd clients over to rpmsg (and by that drop the existing SMD implementation). All this allows me to implement the new backend and reuse all existing SMD drivers with the new mechanism. RPM Communication has to supported even when IRQs are disabled. The most important use of this communication is to set the wake up time for the CPU subsystem when all the CPUs are powered off. In addition to that, "sleep" votes that are sent by the application processor subsystem to allow system to go into deep sleep modes can only be triggered when the CPU PM domains are power collapsed, drivers do not have a knowledge of when that happens. This has to be done by a platform code that registers for CPU PM domain power_off/on callbacks. Ok, my bad. These two cases are not critical for the SoC supported by this driver. So you are good to go from cpuidle perspective Using rpmsg may be nice for RPM SMD communication, but mutexes need to go away for this driver to be any useful than bare bones active mode resource requests for QCOM SoCs. By not doing that now, we lock ourselves out of using this SMD driver in the near future when CPU PM domains are available in the kernel with an ability to do system low power modes. I hope you would make rpmsg work in IRQ disabled contexts first before porting the SMD driver. Thanks, Lina Changes from v1: - Split up the patch moving core code to rpmsg_core into several commits - Dropped the wrapping struct in rpmsg_core and just added the ops to the public API (but hid the implementation details) - Reordered things to reduce the size of the later patches Bjorn Andersson (17): rpmsg: Enable matching devices with drivers based on DT rpmsg: Name rpmsg devices based on channel id rpmsg: rpmsg_send() operations takes rpmsg_endpoint rpmsg: Make rpmsg_create_ept() take channel_info struct rpmsg: Clean up rpmsg device vs channel naming rpmsg: Introduce indirection table for rpmsg_device operations rpmsg: Move rpmsg_device API to new file rpmsg: Indirection table for rpmsg_endpoint operations rpmsg: Move endpoint related interface to rpmsg core rpmsg: Move helper for finding rpmsg devices to core rpmsg: Split off generic tail of create_channel() rpmsg: Split rpmsg core and virtio backend rpmsg: Hide rpmsg indirection tables rpmsg: virtio: Hide vrp pointer from the public API rpmsg: Move virtio specifics from public header rpmsg: Allow callback to return errors rpmsg: Introduce Qualcomm SMD backend drivers/remoteproc/Kconfig |4 +- drivers/rpmsg/Kconfig | 14 + drivers/rpmsg/Makefile |4 +- drivers/rpmsg/qcom_smd.c| 1434 +++ drivers/rpmsg/rpmsg_core.c | 498 drivers/rpmsg/rpmsg_internal.h | 82 ++ drivers/rpmsg/virtio_rpmsg_bus.c| 487 +--- include/linux/rpmsg.h | 246 +- samples/rpmsg/rpmsg_client_sample.c | 14 +- 9 files changed, 2266 insertions(+), 517 deletions(-) create mode 100644 drivers/rpmsg/qcom_smd.c create mode 100644 drivers/rpmsg/rpmsg_core.c create mode 100644 drivers/rpmsg/rpmsg_internal.h -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html