* Re: [PATCH v8 5/6] PM / devfreq: Add PM QoS support
From: Chanwoo Choi @ 2019-09-26 1:19 UTC (permalink / raw)
To: Leonard Crestez, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Kyungmin Park, MyungJoo Ham,
Alexandre Bailon, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <VI1PR04MB7023573BA3D5C5D521DB689CEE870@VI1PR04MB7023.eurprd04.prod.outlook.com>
On 19. 9. 26. 오전 6:18, Leonard Crestez wrote:
> On 25.09.2019 05:11, Chanwoo Choi wrote:
>> On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
>>> Register notifiers with the PM QoS framework in order to respond to
>>> requests for DEV_PM_QOS_MIN_FREQUENCY and DEV_PM_QOS_MAX_FREQUENCY.
>>>
>>> No notifiers are added by this patch but PM QoS constraints can be
>>> imposed externally (for example from other devices).
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> drivers/devfreq/devfreq.c | 75 +++++++++++++++++++++++++++++++++++++++
>>> include/linux/devfreq.h | 5 +++
>>> 2 files changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index eee403e70c84..784f3e40536a 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -22,15 +22,18 @@
>>> #include <linux/platform_device.h>
>>> #include <linux/list.h>
>>> #include <linux/printk.h>
>>> #include <linux/hrtimer.h>
>>> #include <linux/of.h>
>>> +#include <linux/pm_qos.h>
>>> #include "governor.h"
>>>
>>> #define CREATE_TRACE_POINTS
>>> #include <trace/events/devfreq.h>
>>>
>>> +#define HZ_PER_KHZ 1000
>>> +
>>> static struct class *devfreq_class;
>>>
>>> /*
>>> * devfreq core provides delayed work based load monitoring helper
>>> * functions. Governors can use these or can implement their own
>>> @@ -109,10 +112,11 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
>>> static void get_freq_range(struct devfreq *devfreq,
>>> unsigned long *min_freq,
>>> unsigned long *max_freq)
>>> {
>>> unsigned long *freq_table = devfreq->profile->freq_table;
>>> + unsigned long qos_min_freq, qos_max_freq;
>>>
>>> lockdep_assert_held(&devfreq->lock);
>>>
>>> /*
>>> * Init min/max frequency from freq table.
>>> @@ -125,10 +129,18 @@ static void get_freq_range(struct devfreq *devfreq,
>>> } else {
>>> *min_freq = freq_table[devfreq->profile->max_state - 1];
>>> *max_freq = freq_table[0];
>>> }
>>>
>>> + /* constraints from PM QoS */
>>
>> As I commented on patch4,
>> 'constraints' -> 'Constraint' because first verb have to be used
>> as the sigular verbs.
>
> Already discussed for another patch; I will modify to "Apply constraints
> from PM QoS" instead.
>
>> I prefer to use following comments:
>>
>> /* Constraint minimum/maximum frequency from PM QoS constraints */
>>
>>> + qos_min_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> + DEV_PM_QOS_MIN_FREQUENCY);
>>> + qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> + DEV_PM_QOS_MIN_FREQUENCY);
>>> + *min_freq = max(*min_freq, HZ_PER_KHZ * qos_min_freq);
>>> + *max_freq = min(*max_freq, HZ_PER_KHZ * qos_max_freq);
>>> +
>>> /* constraints from sysfs */
>>> *min_freq = max(*min_freq, devfreq->min_freq);
>>> *max_freq = min(*max_freq, devfreq->max_freq);
>>>
>>> /* constraints from OPP interface */
>>> @@ -606,10 +618,49 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>>> mutex_unlock(&devfreq->lock);
>>>
>>> return ret;
>>> }
>>>
>>> +/**
>>> + * qos_notifier_call() - Common handler for QoS constraints.
>>> + * @devfreq: the devfreq instance.
>>> + */
>>> +static int qos_notifier_call(struct devfreq *devfreq)
>>> +{
>>> + int err;
>>> +
>>> + mutex_lock(&devfreq->lock);
>>> + err = update_devfreq(devfreq);
>>> + mutex_unlock(&devfreq->lock);
>>> + if (err)
>>> + dev_err(devfreq->dev.parent,
>>> + "failed to update frequency for PM QoS constraints (%d)\n",
>>
>> Is it not over 80 char?
>
> Yes but coding style explicitly forbids breaking strings.
I want to make it within 80 char. How about following comment?
dev_err(devfreq->dev.parent,
"failed to update frequency from PM QoS (%d)\n",
>>> + err);
>>> +
>>> + return NOTIFY_OK;
>>> +}
>>> +
>>> +/**
>>> + * qos_min_notifier_call() - Callback for QoS min_freq changes.
>>> + * @nb: Should be devfreq->nb_min
>>> + */
>>> +static int qos_min_notifier_call(struct notifier_block *nb,
>>> + unsigned long val, void *ptr)
>>> +{
>>> + return qos_notifier_call(container_of(nb, struct devfreq, nb_min));
>>> +}
>>> +
>>> +/**
>>> + * qos_max_notifier_call() - Callback for QoS max_freq changes.
>>> + * @nb: Should be devfreq->nb_max
>>> + */
>>> +static int qos_max_notifier_call(struct notifier_block *nb,
>>> + unsigned long val, void *ptr)
>>> +{
>>> + return qos_notifier_call(container_of(nb, struct devfreq, nb_max));
>>> +}
>>> +
>>> /**
>>> * devfreq_dev_release() - Callback for struct device to release the device.
>>> * @dev: the devfreq device
>>> *
>>> * Remove devfreq from the list and release its resources.
>>> @@ -620,10 +671,15 @@ static void devfreq_dev_release(struct device *dev)
>>>
>>> mutex_lock(&devfreq_list_lock);
>>> list_del(&devfreq->node);
>>> mutex_unlock(&devfreq_list_lock);
>>>
>>> + dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_max,
>>> + DEV_PM_QOS_MAX_FREQUENCY);
>>> + dev_pm_qos_remove_notifier(devfreq->dev.parent, &devfreq->nb_min,
>>> + DEV_PM_QOS_MIN_FREQUENCY);
>>> +
>>
>> Just print error with dev_err() without stopping the release step.
>>
>> I prefer to handle the return value if kernel API provides
>> the error code.
>>
>>> if (devfreq->profile->exit)
>>> devfreq->profile->exit(devfreq->dev.parent);
>>>
>>> kfree(devfreq->time_in_state);
>>> kfree(devfreq->trans_table);
>>> @@ -733,10 +789,28 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>> if (err) {
>>> put_device(&devfreq->dev);
>>> goto err_out;
>>> }
>>>
>>> + /*
>>> + * Register notifiers for updates to min/max_freq after device is
>>> + * initialized (and we can handle notifications) but before the
>>> + * governor is started (which should do an initial enforcement of
>>> + * constraints).
>>> + */
>>
>> My previous comment is not enough why I prefer to remove it. Sorry.
>> Actually, until now, the devfreq_add_device() don't have the detailed
>> comments because the line code is not too long. But, at the present time,
>> devfreq_add_device() is too long. It means that the detailed comment
>> are necessary.
>>
>> So, I'll add the detailed comment for each step of devfreq_add_device()
>> on separate patch to keep the same style. I'll send the patch to you
>> for the review.
>
> This is very likely to result in merge conflicts, maybe wait for my
> series to go in first?
>
>>> + devfreq->nb_min.notifier_call = qos_min_notifier_call;
>>> + err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
>>> + DEV_PM_QOS_MIN_FREQUENCY);
>>> + if (err)
>>> + goto err_devfreq;
>>> +
>>> + devfreq->nb_max.notifier_call = qos_max_notifier_call;
>>> + err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_max,
>>> + DEV_PM_QOS_MAX_FREQUENCY);
>>> + if (err)
>>> + goto err_devfreq;
>>> +
>>> mutex_lock(&devfreq_list_lock);
>>>
>>> governor = try_then_request_governor(devfreq->governor_name);
>>> if (IS_ERR(governor)) {
>>> dev_err(dev, "%s: Unable to find governor for the device\n",
>>> @@ -760,10 +834,11 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>>
>>> return devfreq;
>>>
>>> err_init:
>>> mutex_unlock(&devfreq_list_lock);
>>> +err_devfreq:
>>> devfreq_remove_device(devfreq);
>>> return ERR_PTR(err);
>>>
>>> err_dev:
>>> /*
>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>> index c3cbc15fdf08..dac0dffeabb4 100644
>>> --- a/include/linux/devfreq.h
>>> +++ b/include/linux/devfreq.h
>>> @@ -134,10 +134,12 @@ struct devfreq_dev_profile {
>>> * @total_trans: Number of devfreq transitions
>>> * @trans_table: Statistics of devfreq transitions
>>> * @time_in_state: Statistics of devfreq states
>>> * @last_stat_updated: The last time stat updated
>>> * @transition_notifier_list: list head of DEVFREQ_TRANSITION_NOTIFIER notifier
>>> + * @nb_min: Notifier block for DEV_PM_QOS_MIN_FREQUENCY
>>> + * @nb_max: Notifier block for DEV_PM_QOS_MAX_FREQUENCY
>>> *
>>> * This structure stores the devfreq information for a give device.
>>> *
>>> * Note that when a governor accesses entries in struct devfreq in its
>>> * functions except for the context of callbacks defined in struct
>>> @@ -176,10 +178,13 @@ struct devfreq {
>>> unsigned int *trans_table;
>>> unsigned long *time_in_state;
>>> unsigned long last_stat_updated;
>>>
>>> struct srcu_notifier_head transition_notifier_list;
>>> +
>>> + struct notifier_block nb_min;
>>> + struct notifier_block nb_max;
>>> };
>>>
>>> struct devfreq_freqs {
>>> unsigned long old;
>>> unsigned long new;
>>>
>>
>>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 6/6] PM / devfreq: Use PM QoS for sysfs min/max_freq
From: Chanwoo Choi @ 2019-09-26 1:25 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan, linux-pm,
Viresh Kumar, NXP Linux Team, Krzysztof Kozlowski, Lukasz Luba,
Kyungmin Park, MyungJoo Ham, Alexandre Bailon, Leonard Crestez,
Georgi Djakov, linux-arm-kernel, Jacky Bai
In-Reply-To: <20190925164513.GM133864@google.com>
On 19. 9. 26. 오전 1:45, Matthias Kaehlcke wrote:
> On Wed, Sep 25, 2019 at 11:41:07AM +0900, Chanwoo Choi wrote:
>> On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
>>> Switch the handling of min_freq and max_freq from sysfs to use the
>>> dev_pm_qos_request interface.
>>>
>>> Since PM QoS handles frequencies as kHz this change reduces the
>>> precision of min_freq and max_freq. This shouldn't introduce problems
>>> because frequencies which are not an integer number of kHz are likely
>>> not an integer number of Hz either.
>>>
>>> Try to ensure compatibility by rounding min values down and rounding
>>> max values up.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> drivers/devfreq/devfreq.c | 46 ++++++++++++++++++++++++---------------
>>> include/linux/devfreq.h | 9 ++++----
>>> 2 files changed, 33 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 784f3e40536a..8bb7efd821ab 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -137,14 +137,10 @@ static void get_freq_range(struct devfreq *devfreq,
>>> qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> DEV_PM_QOS_MIN_FREQUENCY);
>>> *min_freq = max(*min_freq, HZ_PER_KHZ * qos_min_freq);
>>> *max_freq = min(*max_freq, HZ_PER_KHZ * qos_max_freq);
>>>
>>> - /* constraints from sysfs */
>>> - *min_freq = max(*min_freq, devfreq->min_freq);
>>> - *max_freq = min(*max_freq, devfreq->max_freq);
>>> -
>>> /* constraints from OPP interface */
>>> *min_freq = max(*min_freq, devfreq->scaling_min_freq);
>>> /* scaling_max_freq can be zero on error */
>>> if (devfreq->scaling_max_freq)
>>> *max_freq = min(*max_freq, devfreq->scaling_max_freq);
>>> @@ -679,10 +675,12 @@ static void devfreq_dev_release(struct device *dev)
>>> DEV_PM_QOS_MIN_FREQUENCY);
>>>
>>> if (devfreq->profile->exit)
>>> devfreq->profile->exit(devfreq->dev.parent);
>>>
>>> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
>>> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
>>
>> Please check the return value if error happen, just print the err with dev_err()
>> without stopping the release steps.
>
> I wonder if dev_warn() would be more appropriate, since the current operation
> is not aborted.
>
>>> kfree(devfreq->time_in_state);
>>> kfree(devfreq->trans_table);
>>> mutex_destroy(&devfreq->lock);
>>> kfree(devfreq);
>>> }
>>> @@ -747,18 +745,25 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>> devfreq->scaling_min_freq = find_available_min_freq(devfreq);
>>> if (!devfreq->scaling_min_freq) {
>>> err = -EINVAL;
>>> goto err_dev;
>>> }
>>> - devfreq->min_freq = devfreq->scaling_min_freq;
>>>
>>> devfreq->scaling_max_freq = find_available_max_freq(devfreq);
>>> if (!devfreq->scaling_max_freq) {
>>> err = -EINVAL;
>>> goto err_dev;
>>> }
>>> - devfreq->max_freq = devfreq->scaling_max_freq;
>>> +
>>> + err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
>>> + DEV_PM_QOS_MIN_FREQUENCY, 0);
>>> + if (err < 0)
>>> + goto err_dev;
>>> + err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
>>> + DEV_PM_QOS_MAX_FREQUENCY, S32_MAX);
>>> + if (err < 0)
>>> + goto err_dev;
>>>
>>> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>>> atomic_set(&devfreq->suspend_count, 0);
>>>
>>> devfreq->trans_table = kzalloc(
>>> @@ -843,10 +848,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>> err_dev:
>>> /*
>>> * Cleanup path for errors that happen before registration.
>>> * Otherwise we rely on devfreq_dev_release.
>>> */
>>> + if (dev_pm_qos_request_active(&devfreq->user_max_freq_req))
>>> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
>>
>> Please check the return value if error happen, just print the err with dev_err()
>> without stopping the release steps.
>>
>> dev_err(... "failed to remove request of DEV_PM_QOS_MAX_FREQUENCY\n");
>
> dev_warn() for the same reason as above?
Actually, I think that is not critical error but need to print the error
So, I think that dev_err() is enough. If this thing is critical,
better to use dev_warn.
>
> I think the message would be better with a slight change:
>
> "failed to remove DEV_PM_QOS_MAX_FREQUENCY request\n"
OK.
>
>>
>>> + if (dev_pm_qos_request_active(&devfreq->user_min_freq_req))
>>> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
>>
>> dev_err(... "failed to remove request of DEV_PM_QOS_MIN_FREQUENCY\n");
>
> ditto
>
>>> kfree(devfreq->time_in_state);
>>> kfree(devfreq->trans_table);
>>> kfree(devfreq);
>>> err_out:
>>> return ERR_PTR(err);
>>> @@ -1407,14 +1416,15 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
>>>
>>> ret = sscanf(buf, "%lu", &value);
>>> if (ret != 1)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&df->lock);
>>> - df->min_freq = value;
>>> - update_devfreq(df);
>>> - mutex_unlock(&df->lock);
>>> + /* round down to kHz for PM QoS */
>>
>> I prefer more detailed description as following:
>>
>> /*
>> * Round down to KHz to decide the proper minimum frequency
>
> it should be kHz, with a lower-case 'k', as in the original comment.
Good.
>
>> * which is closed to user request.
>> */
>
> The comment you suggest doesn't provide any information about why the
> conversion to kHz is done, in this sense the original comment that
> mentions PM QoS provides more value.
>
> With whatever we end up, I suggest to use 'convert' instead of
> 'round down'.
I agree to use 'convert' instead of 'round down'
if some expression indicates the correct meaning.
>
>>> + ret = dev_pm_qos_update_request(&df->user_min_freq_req,
>>> + value / HZ_PER_KHZ);
>>> + if (ret < 0)
>>> + return ret;
>>>
>>> return count;
>>> }
>>>
>>> static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
>>> @@ -1439,19 +1449,19 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>>>
>>> ret = sscanf(buf, "%lu", &value);
>>> if (ret != 1)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&df->lock);
>>> -
>>> - /* Interpret zero as "don't care" */
>>> - if (!value)
>>> - value = ULONG_MAX;
>>> + /* round up to kHz for PM QoS and interpret zero as "don't care" */
>>
>> I think that "don't care" comment style is not good.
>>
>> I referred to the Documentation/ABI/testing/sysfs-class-devfreq file.
>> I prefer more detailed description as following:
>> /*
>> * Round up to KHz to decide the proper maximum frequency
>
> kHz
>
>> * which is closed to user request. If value is zero,
>> * the user does not care.
>
> "the user does not care" is still very casual you didn't like initially.
> How about "A value of zero is interpreted as 'no limit'."?
>
> As for the min freq, I think PM QoS should be mentioned to make clear why
> the conversion to kHz is needed.
Agree. I expect that Leonard will mention that.
>
>> */
>>
>>
>>> + if (value)
>>> + value = DIV_ROUND_UP(value, HZ_PER_KHZ);
>>> + else
>>> + value = S32_MAX;
>>>
>>> - df->max_freq = value;
>>> - update_devfreq(df);
>>> - mutex_unlock(&df->lock);
>>> + ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
>>> + if (ret < 0)
>>> + return ret;
>>>
>>> return count;
>>> }
>>> static DEVICE_ATTR_RW(min_freq);
>>>
>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>> index dac0dffeabb4..7849fe4c666d 100644
>>> --- a/include/linux/devfreq.h
>>> +++ b/include/linux/devfreq.h
>>> @@ -11,10 +11,11 @@
>>> #define __LINUX_DEVFREQ_H__
>>>
>>> #include <linux/device.h>
>>> #include <linux/notifier.h>
>>> #include <linux/pm_opp.h>
>>> +#include <linux/pm_qos.h>
>>>
>>> #define DEVFREQ_NAME_LEN 16
>>>
>>> /* DEVFREQ governor name */
>>> #define DEVFREQ_GOV_SIMPLE_ONDEMAND "simple_ondemand"
>>> @@ -121,12 +122,12 @@ struct devfreq_dev_profile {
>>> * devfreq.nb to the corresponding register notifier call chain.
>>> * @work: delayed work for load monitoring.
>>> * @previous_freq: previously configured frequency value.
>>> * @data: Private data of the governor. The devfreq framework does not
>>> * touch this.
>>> - * @min_freq: Limit minimum frequency requested by user (0: none)
>>> - * @max_freq: Limit maximum frequency requested by user (0: none)
>>> + * @user_min_freq_req: PM QoS min frequency request from user (via sysfs)
>>
>> min -> minimum and then remove parenthesis as following:
>> PM QoS minimum frequency request by user via sysfs
>>
>>> + * @user_max_freq_req: PM QoS max frequency request from user (via sysfs)
>>
>> ditto. max -> maximum
>> PM QoS maximum frequency request by user via sysfs
>>
>>> * @scaling_min_freq: Limit minimum frequency requested by OPP interface
>>> * @scaling_max_freq: Limit maximum frequency requested by OPP interface
>>> * @stop_polling: devfreq polling status of a device.
>>> * @suspend_freq: frequency of a device set during suspend phase.
>>> * @resume_freq: frequency of a device set in resume phase.
>>> @@ -161,12 +162,12 @@ struct devfreq {
>>> unsigned long previous_freq;
>>> struct devfreq_dev_status last_status;
>>>
>>> void *data; /* private data for governors */
>>>
>>> - unsigned long min_freq;
>>> - unsigned long max_freq;
>>> + struct dev_pm_qos_request user_min_freq_req;
>>> + struct dev_pm_qos_request user_max_freq_req;
>>> unsigned long scaling_min_freq;
>>> unsigned long scaling_max_freq;
>>> bool stop_polling;
>>>
>>> unsigned long suspend_freq;
>>>
>>
>>
>> --
>> Best Regards,
>> Chanwoo Choi
>> Samsung Electronics
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 6/6] PM / devfreq: Use PM QoS for sysfs min/max_freq
From: Chanwoo Choi @ 2019-09-26 1:26 UTC (permalink / raw)
To: Leonard Crestez, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Kyungmin Park, MyungJoo Ham,
Alexandre Bailon, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <VI1PR04MB7023A6F6F6DF39FC273F020AEE870@VI1PR04MB7023.eurprd04.prod.outlook.com>
On 19. 9. 26. 오전 7:11, Leonard Crestez wrote:
> On 25.09.2019 05:36, Chanwoo Choi wrote:
>> On 19. 9. 24. 오후 7:11, Leonard Crestez wrote:
>>> Switch the handling of min_freq and max_freq from sysfs to use the
>>> dev_pm_qos_request interface.
>>>
>>> Since PM QoS handles frequencies as kHz this change reduces the
>>> precision of min_freq and max_freq. This shouldn't introduce problems
>>> because frequencies which are not an integer number of kHz are likely
>>> not an integer number of Hz either.
>>>
>>> Try to ensure compatibility by rounding min values down and rounding
>>> max values up.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>>> ---
>>> drivers/devfreq/devfreq.c | 46 ++++++++++++++++++++++++---------------
>>> include/linux/devfreq.h | 9 ++++----
>>> 2 files changed, 33 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 784f3e40536a..8bb7efd821ab 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -137,14 +137,10 @@ static void get_freq_range(struct devfreq *devfreq,
>>> qos_max_freq = dev_pm_qos_read_value(devfreq->dev.parent,
>>> DEV_PM_QOS_MIN_FREQUENCY);
>>> *min_freq = max(*min_freq, HZ_PER_KHZ * qos_min_freq);
>>> *max_freq = min(*max_freq, HZ_PER_KHZ * qos_max_freq);
>>>
>>> - /* constraints from sysfs */
>>> - *min_freq = max(*min_freq, devfreq->min_freq);
>>> - *max_freq = min(*max_freq, devfreq->max_freq);
>>> -
>>> /* constraints from OPP interface */
>>> *min_freq = max(*min_freq, devfreq->scaling_min_freq);
>>> /* scaling_max_freq can be zero on error */
>>> if (devfreq->scaling_max_freq)
>>> *max_freq = min(*max_freq, devfreq->scaling_max_freq);
>>> @@ -679,10 +675,12 @@ static void devfreq_dev_release(struct device *dev)
>>> DEV_PM_QOS_MIN_FREQUENCY);
>>>
>>> if (devfreq->profile->exit)
>>> devfreq->profile->exit(devfreq->dev.parent);
>>>
>>> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
>>> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
>>
>> Please check the return value if error happen, just print the err with dev_err()
>> without stopping the release steps.
>
> OK, will print errors
>
>>> kfree(devfreq->time_in_state);
>>> kfree(devfreq->trans_table);
>>> mutex_destroy(&devfreq->lock);
>>> kfree(devfreq);
>>> }
>>> @@ -747,18 +745,25 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>> devfreq->scaling_min_freq = find_available_min_freq(devfreq);
>>> if (!devfreq->scaling_min_freq) {
>>> err = -EINVAL;
>>> goto err_dev;
>>> }
>>> - devfreq->min_freq = devfreq->scaling_min_freq;
>>>
>>> devfreq->scaling_max_freq = find_available_max_freq(devfreq);
>>> if (!devfreq->scaling_max_freq) {
>>> err = -EINVAL;
>>> goto err_dev;
>>> }
>>> - devfreq->max_freq = devfreq->scaling_max_freq;
>>> +
>>> + err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
>>> + DEV_PM_QOS_MIN_FREQUENCY, 0);
>>> + if (err < 0)
>>> + goto err_dev;
>>> + err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
>>> + DEV_PM_QOS_MAX_FREQUENCY, S32_MAX);
>>> + if (err < 0)
>>> + goto err_dev;
>>>
>>> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>>> atomic_set(&devfreq->suspend_count, 0);
>>>
>>> devfreq->trans_table = kzalloc(
>>> @@ -843,10 +848,14 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>> err_dev:
>>> /*
>>> * Cleanup path for errors that happen before registration.
>>> * Otherwise we rely on devfreq_dev_release.
>>> */
>>> + if (dev_pm_qos_request_active(&devfreq->user_max_freq_req))
>>> + dev_pm_qos_remove_request(&devfreq->user_max_freq_req);
>>
>> Please check the return value if error happen, just print the err with dev_err()
>> without stopping the release steps.
>
> OK, will print errors
>
>>
>> dev_err(... "failed to remove request of DEV_PM_QOS_MAX_FREQUENCY\n");
>>
>>> + if (dev_pm_qos_request_active(&devfreq->user_min_freq_req))
>>> + dev_pm_qos_remove_request(&devfreq->user_min_freq_req);
>>
>> dev_err(... "failed to remove request of DEV_PM_QOS_MIN_FREQUENCY\n");
>>
>>> kfree(devfreq->time_in_state);
>>> kfree(devfreq->trans_table);
>>> kfree(devfreq);
>>> err_out:
>>> return ERR_PTR(err);
>>> @@ -1407,14 +1416,15 @@ static ssize_t min_freq_store(struct device *dev, struct device_attribute *attr,
>>>
>>> ret = sscanf(buf, "%lu", &value);
>>> if (ret != 1)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&df->lock);
>>> - df->min_freq = value;
>>> - update_devfreq(df);
>>> - mutex_unlock(&df->lock);
>>> + /* round down to kHz for PM QoS */
>>
>> I prefer more detailed description as following:
>>
>> /*
>> * Round down to KHz to decide the proper minimum frequency
>> * which is closed to user request.
>> */
How about this comment? and with Matthias comment on other reply thread.
>>
>>
>>> + ret = dev_pm_qos_update_request(&df->user_min_freq_req,
>>> + value / HZ_PER_KHZ);
>>> + if (ret < 0)
>>> + return ret;
>>>
>>> return count;
>>> }
>>>
>>> static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
>>> @@ -1439,19 +1449,19 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>>>
>>> ret = sscanf(buf, "%lu", &value);
>>> if (ret != 1)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&df->lock);
>>> -
>>> - /* Interpret zero as "don't care" */
>>> - if (!value)
>>> - value = ULONG_MAX;
>>> + /* round up to kHz for PM QoS and interpret zero as "don't care" */
>>
>> I think that "don't care" comment style is not good.
>>
>> I referred to the Documentation/ABI/testing/sysfs-class-devfreq file.
>> I prefer more detailed description as following:
>> /*
>> * Round up to KHz to decide the proper maximum frequency
>> * which is closed to user request. If value is zero,
>> * the user does not care.
>> */
>
> OK, will update this comment
>
>>> + if (value)
>>> + value = DIV_ROUND_UP(value, HZ_PER_KHZ);
>>> + else
>>> + value = S32_MAX;
>>>
>>> - df->max_freq = value;
>>> - update_devfreq(df);
>>> - mutex_unlock(&df->lock);
>>> + ret = dev_pm_qos_update_request(&df->user_max_freq_req, value);
>>> + if (ret < 0)
>>> + return ret;
>>>
>>> return count;
>>> }
>>> static DEVICE_ATTR_RW(min_freq);
>>>
>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>> index dac0dffeabb4..7849fe4c666d 100644
>>> --- a/include/linux/devfreq.h
>>> +++ b/include/linux/devfreq.h
>>> @@ -11,10 +11,11 @@
>>> #define __LINUX_DEVFREQ_H__
>>>
>>> #include <linux/device.h>
>>> #include <linux/notifier.h>
>>> #include <linux/pm_opp.h>
>>> +#include <linux/pm_qos.h>
>>>
>>> #define DEVFREQ_NAME_LEN 16
>>>
>>> /* DEVFREQ governor name */
>>> #define DEVFREQ_GOV_SIMPLE_ONDEMAND "simple_ondemand"
>>> @@ -121,12 +122,12 @@ struct devfreq_dev_profile {
>>> * devfreq.nb to the corresponding register notifier call chain.
>>> * @work: delayed work for load monitoring.
>>> * @previous_freq: previously configured frequency value.
>>> * @data: Private data of the governor. The devfreq framework does not
>>> * touch this.
>>> - * @min_freq: Limit minimum frequency requested by user (0: none)
>>> - * @max_freq: Limit maximum frequency requested by user (0: none)
>>> + * @user_min_freq_req: PM QoS min frequency request from user (via sysfs)
>>
>> min -> minimum and then remove parenthesis as following:
>> PM QoS minimum frequency request by user via sysfs
>>
>>> + * @user_max_freq_req: PM QoS max frequency request from user (via sysfs)
>>
>> ditto. max -> maximum
>> PM QoS maximum frequency request by user via sysfs
>
> OK
Thanks.
>
>>> * @scaling_min_freq: Limit minimum frequency requested by OPP interface
>>> * @scaling_max_freq: Limit maximum frequency requested by OPP interface
>>> * @stop_polling: devfreq polling status of a device.
>>> * @suspend_freq: frequency of a device set during suspend phase.
>>> * @resume_freq: frequency of a device set in resume phase.
>>> @@ -161,12 +162,12 @@ struct devfreq {
>>> unsigned long previous_freq;
>>> struct devfreq_dev_status last_status;
>>>
>>> void *data; /* private data for governors */
>>>
>>> - unsigned long min_freq;
>>> - unsigned long max_freq;
>>> + struct dev_pm_qos_request user_min_freq_req;
>>> + struct dev_pm_qos_request user_max_freq_req;
>>> unsigned long scaling_min_freq;
>>> unsigned long scaling_max_freq;
>>> bool stop_polling;
>>>
>>> unsigned long suspend_freq;
>>>
>>
>>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [EXT] Re: [v3,3/3] Documentation: dt: binding: fsl: Add 'fsl,ippdexpcr-alt-addr' property
From: Biwen Li @ 2019-09-26 1:54 UTC (permalink / raw)
To: Leo Li
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, Ran Wang, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CADRPPNQ+=au2qRL2K-tzhH8HK1+sO+ut9YBhYw4UhWSv5FF88A@mail.gmail.com>
> Caution: EXT Email
>
> On Tue, Sep 24, 2019 at 11:27 PM Biwen Li <biwen.li@nxp.com> wrote:
> >
> > > > >
> > > > > > > > > > >
> > > > > > > > > > > The 'fsl,ippdexpcr-alt-addr' property is used to
> > > > > > > > > > > handle an errata
> > > > > > > > > > > A-008646 on LS1021A
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Biwen Li <biwen.li@nxp.com>
> > > > > > > > > > > ---
> > > > > > > > > > > Change in v3:
> > > > > > > > > > > - rename property name
> > > > > > > > > > > fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
> > > > > > > > > > >
> > > > > > > > > > > Change in v2:
> > > > > > > > > > > - update desc of the property 'fsl,rcpm-scfg'
> > > > > > > > > > >
> > > > > > > > > > > Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > | 14
> > > > > > > > > > > ++++++++++++++
> > > > > > > > > > > 1 file changed, 14 insertions(+)
> > > > > > > > > > >
> > > > > > > > > > > diff --git
> > > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > index 5a33619d881d..157dcf6da17c 100644
> > > > > > > > > > > ---
> > > > > > > > > > > a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
> > > > > > > > > > > +++ b/Documentation/devicetree/bindings/soc/fsl/rcpm
> > > > > > > > > > > +++ .txt
> > > > > > > > > > > @@ -34,6 +34,11 @@ Chassis Version Example
> > > > > Chips
> > > > > > > > > > > Optional properties:
> > > > > > > > > > > - little-endian : RCPM register block is Little Endian.
> > > > > > > > > > > Without it
> > > > > > RCPM
> > > > > > > > > > > will be Big Endian (default case).
> > > > > > > > > > > + - fsl,ippdexpcr-alt-addr : Must add the property
> > > > > > > > > > > + for SoC LS1021A,
> > > > > > > > > >
> > > > > > > > > > You probably should mention this is related to a
> > > > > > > > > > hardware issue on LS1021a and only needed on LS1021a.
> > > > > > > > > Okay, got it, thanks, I will add this in v4.
> > > > > > > > > >
> > > > > > > > > > > + Must include n + 1 entries (n =
> > > > > > > > > > > + #fsl,rcpm-wakeup-cells, such
> > > > as:
> > > > > > > > > > > + #fsl,rcpm-wakeup-cells equal to 2, then must
> > > > > > > > > > > + include
> > > > > > > > > > > + 2
> > > > > > > > > > > + +
> > > > > > > > > > > + 1
> > > > > > entries).
> > > > > > > > > >
> > > > > > > > > > #fsl,rcpm-wakeup-cells is the number of IPPDEXPCR
> > > > > > > > > > registers on an
> > > > > > SoC.
> > > > > > > > > > However you are defining an offset to scfg registers here.
> > > > > > > > > > Why these two are related? The length here should
> > > > > > > > > > actually be related to the #address-cells of the soc/.
> > > > > > > > > > But since this is only needed for LS1021, you can
> > > > > > > > > just make it 3.
> > > > > > > > > I need set the value of IPPDEXPCR resgiters from
> > > > > > > > > ftm_alarm0 device node(fsl,rcpm-wakeup = <&rcpm 0x0
> > > > > > > > > 0x20000000>;
> > > > > > > > > 0x0 is a value for IPPDEXPCR0, 0x20000000 is a value for
> > > > > > IPPDEXPCR1).
> > > > > > > > > But because of the hardware issue on LS1021A, I need
> > > > > > > > > store the value of IPPDEXPCR registers to an alt
> > > > > > > > > address. So I defining an offset to scfg registers, then
> > > > > > > > > RCPM driver get an abosolute address from offset, RCPM
> > > > > > > > > driver write the value of IPPDEXPCR registers to these
> > > > > > > > > abosolute addresses(backup the value of IPPDEXPCR
> > > > > > registers).
> > > > > > > >
> > > > > > > > I understand what you are trying to do. The problem is
> > > > > > > > that the new fsl,ippdexpcr-alt-addr property contains a
> > > > > > > > phandle and an
> > > offset.
> > > > > > > > The size of it shouldn't be related to #fsl,rcpm-wakeup-cells.
> > > > > > > You maybe like this: fsl,ippdexpcr-alt-addr = <&scfg
> > > > > > > 0x51c>;/*
> > > > > > > SCFG_SPARECR8 */
> > > > > >
> > > > > > No. The #address-cell for the soc/ is 2, so the offset to
> > > > > > scfg should be 0x0 0x51c. The total size should be 3, but it
> > > > > > shouldn't be coming from #fsl,rcpm-wakeup-cells like you
> > > > > > mentioned in the
> > > binding.
> > > > > Oh, I got it. You want that fsl,ippdexpcr-alt-add is relative
> > > > > with #address-cells instead of #fsl,rcpm-wakeup-cells.
> > > >
> > > > Yes.
> > > I got an example from drivers/pci/controller/dwc/pci-layerscape.c
> > > and arch/arm/boot/dts/ls1021a.dtsi as follows:
> > > fsl,pcie-scfg = <&scfg 0>, 0 is an index
> > >
> > > In my fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x51c>, It means that 0x0
> > > is an alt offset address for IPPDEXPCR0, 0x51c is an alt offset
> > > address For
> > > IPPDEXPCR1 instead of 0x0 and 0x51c compose to an alt address of
> > > SCFG_SPARECR8.
> > Maybe I need write it as:
> > fsl,ippdexpcr-alt-addr = <&scfg 0x0 0x0 0x0 0x51c>; first two 0x0
> > compose an alt offset address for IPPDEXPCR0, last 0x0 and 0x51c
> > compose an alt address for IPPDEXPCR1,
>
> I remember the hardware issue is only is only related to IPPDEXPCR1 register, no
> idea why you need to define IPPDEXPCR0 in the binding.
Okay, got it, thanks.
Best Regards,
Biwen Li
>
> >
> > Best Regards,
> > Biwen Li
> > > >
> > > > Regards,
> > > > Leo
> > > > > >
> > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > + The first entry must be a link to the SCFG device node.
> > > > > > > > > > > + The non-first entry must be offset of registers of SCFG.
> > > > > > > > > > >
> > > > > > > > > > > Example:
> > > > > > > > > > > The RCPM node for T4240:
> > > > > > > > > > > @@ -43,6 +48,15 @@ The RCPM node for T4240:
> > > > > > > > > > > #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > > > };
> > > > > > > > > > >
> > > > > > > > > > > +The RCPM node for LS1021A:
> > > > > > > > > > > + rcpm: rcpm@1ee2140 {
> > > > > > > > > > > + compatible = "fsl,ls1021a-rcpm",
> > > > > > > > > > > +"fsl,qoriq-rcpm-
> > > > > > > 2.1+";
> > > > > > > > > > > + reg = <0x0 0x1ee2140 0x0 0x8>;
> > > > > > > > > > > + #fsl,rcpm-wakeup-cells = <2>;
> > > > > > > > > > > + fsl,ippdexpcr-alt-addr = <&scfg 0x0
> > > > > > > > > > > + 0x51c>; /*
> > > > > > > > > > > SCFG_SPARECR8 */
> > > > > > > > > > > + };
> > > > > > > > > > > +
> > > > > > > > > > > +
> > > > > > > > > > > * Freescale RCPM Wakeup Source Device Tree Bindings
> > > > > > > > > > > -------------------------------------------
> > > > > > > > > > > Required fsl,rcpm-wakeup property should be added
> > > > > > > > > > > to a device node if the device
> > > > > > > > > > > --
> > > > > > > > > > > 2.17.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 0/6] arm64: meson-gx: misc fixes and updates
From: Kevin Hilman @ 2019-09-26 2:09 UTC (permalink / raw)
To: Christian Hewitt, Rob Herring, Mark Rutland, devicetree,
linux-arm-kernel, linux-amlogic, linux-kernel
Cc: Chrisitian Hewitt
In-Reply-To: <1568041287-7805-1-git-send-email-christianshewitt@gmail.com>
Christian Hewitt <christianshewitt@gmail.com> writes:
> This patchset:
>
> - Fixes bluetooth on Khadas VIM2
> - Fixes bluetooth on Khadas VIM
> - Fixes GPIO key dt on Khadas VIM
> - Updates model for AML-S805X-CC
> - Updates model/compatible for AML-S905X-CC
Queued for v5.5.
Thanks for the updates/fixups,
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [v4, 3/3] Documentation: dt: binding: fsl: Add 'fsl, ippdexpcr1-alt-addr' property
From: Biwen Li @ 2019-09-26 2:41 UTC (permalink / raw)
To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
Cc: devicetree, linuxppc-dev, linux-kernel, linux-arm-kernel,
Biwen Li
In-Reply-To: <20190926024118.15931-1-biwen.li@nxp.com>
The 'fsl,ippdexpcr1-alt-addr' property is used to handle an errata A-008646
on LS1021A
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v4:
- rename property name
fsl,ippdexpcr-alt-addr -> fsl,ippdexpcr1-alt-addr
Change in v3:
- rename property name
fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
Change in v2:
- update desc of the property 'fsl,rcpm-scfg'
.../devicetree/bindings/soc/fsl/rcpm.txt | 21 +++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
index 5a33619d881d..751a7655b694 100644
--- a/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+++ b/Documentation/devicetree/bindings/soc/fsl/rcpm.txt
@@ -34,6 +34,13 @@ Chassis Version Example Chips
Optional properties:
- little-endian : RCPM register block is Little Endian. Without it RCPM
will be Big Endian (default case).
+ - fsl,ippdexpcr1-alt-addr : The property is related to a hardware issue
+ on SoC LS1021A and only needed on SoC LS1021A.
+ Must include 1 + 2 entries.
+ The first entry must be a link to the SCFG device node.
+ The non-first entry must be offset of registers of SCFG.
+ The second and third entry compose an alt offset address
+ for IPPDEXPCR1(SCFG_SPARECR8)
Example:
The RCPM node for T4240:
@@ -43,6 +50,20 @@ The RCPM node for T4240:
#fsl,rcpm-wakeup-cells = <2>;
};
+The RCPM node for LS1021A:
+ rcpm: rcpm@1ee2140 {
+ compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
+ reg = <0x0 0x1ee2140 0x0 0x8>;
+ #fsl,rcpm-wakeup-cells = <2>;
+
+ /*
+ * The second and third entry compose an alt offset
+ * address for IPPDEXPCR1(SCFG_SPARECR8)
+ */
+ fsl,ippdexpcr1-alt-addr = <&scfg 0x0 0x51c>;
+ };
+
+
* Freescale RCPM Wakeup Source Device Tree Bindings
-------------------------------------------
Required fsl,rcpm-wakeup property should be added to a device node if the device
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [v4,1/3] soc: fsl: handle RCPM errata A-008646 on SoC LS1021A
From: Biwen Li @ 2019-09-26 2:41 UTC (permalink / raw)
To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
Cc: devicetree, linuxppc-dev, linux-kernel, linux-arm-kernel,
Biwen Li
Description:
- Reading configuration register RCPM_IPPDEXPCR1
always return zero
Workaround:
- Save register RCPM_IPPDEXPCR1's value to
register SCFG_SPARECR8.(uboot's psci also
need reading value from the register SCFG_SPARECR8
to set register RCPM_IPPDEXPCR1)
Impact:
- FlexTimer module will cannot wakeup system in
deep sleep on SoC LS1021A
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v4:
- rename property name
fsl,ippdexpcr-alt-addr -> fsl,ippdexpcr1-alt-addr
Change in v3:
- update commit message
- rename property name
fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
Change in v2:
- fix stype problems
drivers/soc/fsl/rcpm.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
index 82c0ad5e663e..9a29c482fc2e 100644
--- a/drivers/soc/fsl/rcpm.c
+++ b/drivers/soc/fsl/rcpm.c
@@ -13,6 +13,8 @@
#include <linux/slab.h>
#include <linux/suspend.h>
#include <linux/kernel.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
#define RCPM_WAKEUP_CELL_MAX_SIZE 7
@@ -29,6 +31,9 @@ static int rcpm_pm_prepare(struct device *dev)
struct rcpm *rcpm;
u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp;
int i, ret, idx;
+ struct regmap *scfg_addr_regmap = NULL;
+ u32 reg_offset[RCPM_WAKEUP_CELL_MAX_SIZE + 1];
+ u32 reg_value = 0;
rcpm = dev_get_drvdata(dev);
if (!rcpm)
@@ -63,6 +68,34 @@ static int rcpm_pm_prepare(struct device *dev)
tmp |= value[i + 1];
iowrite32be(tmp, rcpm->ippdexpcr_base + i * 4);
}
+ /* Workaround of errata A-008646 on SoC LS1021A:
+ * There is a bug of register ippdexpcr1.
+ * Reading configuration register RCPM_IPPDEXPCR1
+ * always return zero. So save ippdexpcr1's value
+ * to register SCFG_SPARECR8.And the value of
+ * ippdexpcr1 will be read from SCFG_SPARECR8.
+ */
+ scfg_addr_regmap = syscon_regmap_lookup_by_phandle(np,
+ "fsl,ippdexpcr1-alt-addr");
+ if (scfg_addr_regmap && (1 == i)) {
+ if (of_property_read_u32_array(dev->of_node,
+ "fsl,ippdexpcr1-alt-addr",
+ reg_offset,
+ 1 + sizeof(u64)/sizeof(u32))) {
+ scfg_addr_regmap = NULL;
+ continue;
+ }
+ /* Read value from register SCFG_SPARECR8 */
+ regmap_read(scfg_addr_regmap,
+ (u32)(((u64)(reg_offset[1] << (sizeof(u32) * 8) |
+ reg_offset[2])) & 0xffffffff),
+ ®_value);
+ /* Write value to register SCFG_SPARECR8 */
+ regmap_write(scfg_addr_regmap,
+ (u32)(((u64)(reg_offset[1] << (sizeof(u32) * 8) |
+ reg_offset[2])) & 0xffffffff),
+ tmp | reg_value);
+ }
}
}
} while (ws = wakeup_source_get_next(ws));
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [v4, 2/3] arm: dts: ls1021a: fix that FlexTimer cannot wakeup system in deep sleep
From: Biwen Li @ 2019-09-26 2:41 UTC (permalink / raw)
To: leoyang.li, shawnguo, robh+dt, mark.rutland, ran.wang_1
Cc: devicetree, linuxppc-dev, linux-kernel, linux-arm-kernel,
Biwen Li
In-Reply-To: <20190926024118.15931-1-biwen.li@nxp.com>
The patch fixes a bug that FlexTimer cannot
wakeup system in deep sleep.
Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v4:
- update property name
fsl,ippdexpcr-alt-addr -> fsl,ippdexpcr1-alt-addr
Change in v3:
- update property name
fsl,rcpm-scfg -> fsl,ippdexpcr-alt-addr
Change in v2:
- None
arch/arm/boot/dts/ls1021a.dtsi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/boot/dts/ls1021a.dtsi b/arch/arm/boot/dts/ls1021a.dtsi
index e3973b611c3a..ae427f039e8b 100644
--- a/arch/arm/boot/dts/ls1021a.dtsi
+++ b/arch/arm/boot/dts/ls1021a.dtsi
@@ -1000,6 +1000,12 @@
compatible = "fsl,ls1021a-rcpm", "fsl,qoriq-rcpm-2.1+";
reg = <0x0 0x1ee2140 0x0 0x8>;
#fsl,rcpm-wakeup-cells = <2>;
+
+ /*
+ * The second and third entry compose an alt offset
+ * address for IPPDEXPCR1(SCFG_SPARECR8)
+ */
+ fsl,ippdexpcr1-alt-addr = <&scfg 0x0 0x51c>;
};
ftm_alarm0: timer0@29d0000 {
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device
From: Lokesh Vutla @ 2019-09-26 3:33 UTC (permalink / raw)
To: Tero Kristo, Nishanth Menon, Santosh Shilimkar
Cc: Sekhar Nori, Linux ARM Mailing List
In-Reply-To: <ee2df4ea-b414-8c20-9a55-fa759673806a@ti.com>
Hi Tero,
On 24/09/19 10:15 AM, Lokesh Vutla wrote:
> Hi Tero,
>
> On 23/09/19 12:07 PM, Tero Kristo wrote:
>> On 23/09/2019 06:34, Lokesh Vutla wrote:
>>> Device ID that is passed from power-domains is used by peripheral
>>> drivers for communicating with sysfw. Instead of individual drivers
>>> traversing power-domains entry in DT node, store the device ID in
>>> platform_device so that drivers can directly access it.
>>
>> Uhm, isn't this kind of wrong place to allocate the ID? The power domain
>
> I do agree that this might not be a right place, but I couldn't find a better
> place to populate id. Below is the flow on how platform_device gets created.
> of_platform_default_populate_init
> ->of_platform_default_populate
> -> of_platform_populate
> ->of_platform_bus_create
> -> of_platform_device_create_pdata
> -> of_device_alloc
> -> platform_device_alloc("", PLATFORM_DEVID_NONE);
>
> At this point platform_device gets created with dummy device_id. Also there are
> no hooks to add custom device_ids.
>
>> solution itself is a client also. In theory, someone could access the pdev->id
>
> Nope, this is done in dev_pm_domain_attach which is called before driver probe
> in platform_drv_probe().
If there are no objections, can this patch be picked?
Thanks and regards,
Lokesh
>
>> before this. pdev->id should be assigned by bus driver so that it can be
>> properly handled within platform_device_add.
>
> DT doesn't provide any such facility for populating device_add. I am open for
> any suggestions :)
>
> Thanks and regards,
> Lokesh
>
>>
>> -Tero
>>
>>>
>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>> ---
>>> drivers/soc/ti/ti_sci_pm_domains.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c
>>> b/drivers/soc/ti/ti_sci_pm_domains.c
>>> index 8c2a2f23982c..a124ac409124 100644
>>> --- a/drivers/soc/ti/ti_sci_pm_domains.c
>>> +++ b/drivers/soc/ti/ti_sci_pm_domains.c
>>> @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain
>>> *domain,
>>> struct of_phandle_args pd_args;
>>> struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain);
>>> const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci;
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> struct ti_sci_genpd_dev_data *sci_dev_data;
>>> struct generic_pm_domain_data *genpd_data;
>>> int idx, ret = 0;
>>> @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain
>>> *domain,
>>> return -EINVAL;
>>> idx = pd_args.args[0];
>>> + pdev->id = idx;
>>> /*
>>> * Check the validity of the requested idx, if the index is not valid
>>>
>>
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/3] dt-bindings: reset: add bindings for the Meson-A1 SoC Reset Controller
From: Xingyu Chen @ 2019-09-26 3:40 UTC (permalink / raw)
To: Kevin Hilman, Philipp Zabel, Neil Armstrong
Cc: devicetree, Hanjie Lin, Jianxin Pan, linux-kernel, Rob Herring,
linux-amlogic, linux-arm-kernel, Jerome Brunet
In-Reply-To: <7htv90rnp2.fsf@baylibre.com>
Hi, Kevin
Thanks for your review
On 2019/9/26 6:55, Kevin Hilman wrote:
> Xingyu Chen <xingyu.chen@amlogic.com> writes:
>
>> Add DT bindings for the Meson-A1 SoC Reset Controller include file,
>> and also slightly update documentation.
>>
>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>
> The order here doesn't look right. As the sender, your sign-off should
> be last. Is Jianxin the author or are you? If Jianxin, there should be
> a "From:" line at the beginning of the changelog to indicate authorship
> that's different from the sender.
I am an author for this patchset, i will reorder Signed-off in next version.
>
>> ---
>> .../bindings/reset/amlogic,meson-reset.yaml | 1 +
>> include/dt-bindings/reset/amlogic,meson-a1-reset.h | 59 ++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>> create mode 100644 include/dt-bindings/reset/amlogic,meson-a1-reset.h
>>
>> diff --git a/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
>> index 00917d8..b3f57d8 100644
>> --- a/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
>> +++ b/Documentation/devicetree/bindings/reset/amlogic,meson-reset.yaml
>> @@ -16,6 +16,7 @@ properties:
>> - amlogic,meson8b-reset # Reset Controller on Meson8b and compatible SoCs
>> - amlogic,meson-gxbb-reset # Reset Controller on GXBB and compatible SoCs
>> - amlogic,meson-axg-reset # Reset Controller on AXG and compatible SoCs
>> + - amlogic,meson-a1-reset # Reset Controller on A1 and compatible SoCs
>>
>> reg:
>> maxItems: 1
>> diff --git a/include/dt-bindings/reset/amlogic,meson-a1-reset.h b/include/dt-bindings/reset/amlogic,meson-a1-reset.h
>> new file mode 100644
>> index 00000000..8d76a47
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/amlogic,meson-a1-reset.h
>> @@ -0,0 +1,59 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> + *
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + * Author: Xingyu Chen <xingyu.chen@amlogic.com>
>> + *
>> + */
>> +
>> +#ifndef _DT_BINDINGS_AMLOGIC_MESON_A1_RESET_H
>> +#define _DT_BINDINGS_AMLOGIC_MESON_A1_RESET_H
>> +
>> +/* RESET0 */
>> +#define RESET_AM2AXI_VAD 1
>
> minor nit: can you use comments/whitespace here to indicate holes?
> Please see the other amlogic files in this dir for examples.
I will fix it in next version.
>
> Kevin
>
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/3] reset: add support for the Meson-A1 SoC Reset Controller
From: Xingyu Chen @ 2019-09-26 3:44 UTC (permalink / raw)
To: Kevin Hilman, Philipp Zabel, Neil Armstrong
Cc: Hanjie Lin, Jianxin Pan, linux-kernel, Rob Herring, linux-amlogic,
linux-arm-kernel, Jerome Brunet
In-Reply-To: <7hlfucrnlo.fsf@baylibre.com>
Hi, Kevin
Thanks for your reminder
On 2019/9/26 6:57, Kevin Hilman wrote:
> Hi Xingyu,
>
> Xingyu Chen <xingyu.chen@amlogic.com> writes:
>
>> The number of RESET registers and offset of RESET_LEVEL register for
>> Meson-A1 are different from previous SoCs, In order to describe these
>> differences, we introduce the struct meson_reset_param.
>>
>> Signed-off-by: Xingyu Chen <xingyu.chen@amlogic.com>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Again, order here isn't quite right. Add the reviewed-by tags first,
> and the sender should be the last sign-off.
I will reorder Signed-off and Reviewed in next version
>
> Other than that, driver looks good.
>
> Reviewed-by: Kevin Hilman <khilman@baylibre.com >
> Kevin
>
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
From: Rayagonda Kokatanur @ 2019-09-26 4:40 UTC (permalink / raw)
To: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Wolfram Sang,
Michael Cheng, Shreesha Rajashekar, Lori Hikichi, linux-i2c,
linux-arm-kernel, linux-kernel
Cc: Rayagonda Kokatanur, Icarus Chau, Ray Jui, Shivaraj Shetty
From: Lori Hikichi <lori.hikichi@broadcom.com>
Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.
Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
---
changes from v1:
- Address code review comment from Wolfram Sang
drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++++++++++++++++++++++++++++++--------
1 file changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..e478db7 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
#define M_CMD_PROTOCOL_MASK 0xf
#define M_CMD_PROTOCOL_BLK_WR 0x7
#define M_CMD_PROTOCOL_BLK_RD 0x8
+#define M_CMD_PROTOCOL_PROCESS 0xa
#define M_CMD_PEC_SHIFT 8
#define M_CMD_RD_CNT_SHIFT 0
#define M_CMD_RD_CNT_MASK 0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
return 0;
}
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
- struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+ struct i2c_msg *msgs, bool process_call)
{
int i;
u8 addr;
u32 val, tmp, val_intr_en;
unsigned int tx_bytes;
+ struct i2c_msg *msg = &msgs[0];
/* check if bus is busy */
if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
val = msg->buf[i];
/* mark the last byte */
- if (i == msg->len - 1)
- val |= BIT(M_TX_WR_STATUS_SHIFT);
+ if (!process_call && (i == msg->len - 1))
+ val |= 1 << M_TX_WR_STATUS_SHIFT;
iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
}
iproc_i2c->tx_bytes = tx_bytes;
}
+ /* Process the read message if this is process call */
+ if (process_call) {
+ msg++;
+ iproc_i2c->msg = msg; /* point to second msg */
+
+ /*
+ * The last byte to be sent out should be a slave
+ * address with read operation
+ */
+ addr = i2c_8bit_addr_from_msg(msg);
+ /* mark it the last byte out */
+ val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+ iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+ }
+
/* mark as incomplete before starting the transaction */
if (iproc_i2c->irq)
reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
* underrun interrupt, which will be triggerred when the TX FIFO is
* empty. When that happens we can then pump more data into the FIFO
*/
- if (!(msg->flags & I2C_M_RD) &&
+ if (!process_call && !(msg->flags & I2C_M_RD) &&
msg->len > iproc_i2c->tx_bytes)
val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
*/
val = BIT(M_CMD_START_BUSY_SHIFT);
if (msg->flags & I2C_M_RD) {
+ u32 protocol;
+
iproc_i2c->rx_bytes = 0;
if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
/* enable the RX threshold interrupt */
val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
- val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+ protocol = process_call ?
+ M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+ val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
(msg->len << M_CMD_RD_CNT_SHIFT);
} else {
val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
@@ -774,17 +802,24 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
struct i2c_msg msgs[], int num)
{
struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
- int ret, i;
+ bool process_call = false;
+ int ret;
- /* go through all messages */
- for (i = 0; i < num; i++) {
- ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
- if (ret) {
- dev_dbg(iproc_i2c->device, "xfer failed\n");
- return ret;
+ if (num == 2) {
+ /* Repeated start, use process call */
+ process_call = true;
+ if (msgs[1].flags & I2C_M_NOSTART) {
+ dev_err(iproc_i2c->device, "Invalid repeated start\n");
+ return -EOPNOTSUPP;
}
}
+ ret = bcm_iproc_i2c_xfer_internal(iproc_i2c, msgs, process_call);
+ if (ret) {
+ dev_dbg(iproc_i2c->device, "xfer failed\n");
+ return ret;
+ }
+
return num;
}
@@ -806,6 +841,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
};
static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
+ .flags = I2C_AQ_COMB_WRITE_THEN_READ,
+ .max_comb_1st_msg_len = M_TX_RX_FIFO_SIZE,
.max_read_len = M_RX_MAX_READ_LEN,
};
--
1.9.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [LINUX PATCH] dma-mapping: Control memset operation using gfp flags
From: Dylan Yip @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, iommu, satishna; +Cc: Dylan Yip
In case of 4k video buffer, the allocation from a reserved memory is
taking a long time, ~500ms. This is root caused to the memset()
operations on the allocated memory which is consuming more cpu cycles.
Due to this delay, we see that initial frames are being dropped.
To fix this, we have wrapped the default memset, done when allocating
coherent memory, under the __GFP_ZERO flag. So, we only clear
allocated memory if __GFP_ZERO flag is enabled. We believe this
should be safe as the video decoder always writes before reading.
This optimizes decoder initialization as we do not set the __GFP_ZERO
flag when allocating memory for decoder. With this optimization, we
don't see initial frame drops and decoder initialization time is
~100ms.
This patch adds plumbing through dma_alloc functions to pass gfp flag
set by user to __dma_alloc_from_coherent(). Here gfp flag is checked
for __GFP_ZERO. If present, we memset the buffer to 0 otherwise we
skip memset.
Signed-off-by: Dylan Yip <dylan.yip@xilinx.com>
---
arch/arm/mm/dma-mapping-nommu.c | 2 +-
include/linux/dma-mapping.h | 11 +++++++----
kernel/dma/coherent.c | 15 +++++++++------
kernel/dma/mapping.c | 2 +-
4 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index 52b8255..242b2c3 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
unsigned long attrs)
{
- void *ret = dma_alloc_from_global_coherent(size, dma_handle);
+ void *ret = dma_alloc_from_global_coherent(size, dma_handle, gfp);
/*
* dma_alloc_from_global_coherent() may fail because:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea..b715c9f 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -160,24 +160,27 @@ static inline int is_device_dma_capable(struct device *dev)
* Don't use them in device drivers.
*/
int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
- dma_addr_t *dma_handle, void **ret);
+ dma_addr_t *dma_handle, void **ret,
+ gfp_t flag);
int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, size_t size, int *ret);
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
+void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle,
+ gfp_t flag);
int dma_release_from_global_coherent(int order, void *vaddr);
int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
size_t size, int *ret);
#else
-#define dma_alloc_from_dev_coherent(dev, size, handle, ret) (0)
+#define dma_alloc_from_dev_coherent(dev, size, handle, ret, flag) (0)
#define dma_release_from_dev_coherent(dev, order, vaddr) (0)
#define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
static inline void *dma_alloc_from_global_coherent(ssize_t size,
- dma_addr_t *dma_handle)
+ dma_addr_t *dma_handle,
+ gfp_t flag)
{
return NULL;
}
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 29fd659..d85fab5 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -136,7 +136,7 @@ void dma_release_declared_memory(struct device *dev)
EXPORT_SYMBOL(dma_release_declared_memory);
static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
- ssize_t size, dma_addr_t *dma_handle)
+ ssize_t size, dma_addr_t *dma_handle, gfp_t gfp_flag)
{
int order = get_order(size);
unsigned long flags;
@@ -158,7 +158,8 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
ret = mem->virt_base + (pageno << PAGE_SHIFT);
spin_unlock_irqrestore(&mem->spinlock, flags);
- memset(ret, 0, size);
+ if (gfp_flag & __GFP_ZERO)
+ memset(ret, 0, size);
return ret;
err:
spin_unlock_irqrestore(&mem->spinlock, flags);
@@ -172,6 +173,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
* @dma_handle: This will be filled with the correct dma handle
* @ret: This pointer will be filled with the virtual address
* to allocated area.
+ * @flag: gfp flag set by user
*
* This function should be only called from per-arch dma_alloc_coherent()
* to support allocation from per-device coherent memory pools.
@@ -180,24 +182,25 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
* generic memory areas, or !0 if dma_alloc_coherent should return @ret.
*/
int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
- dma_addr_t *dma_handle, void **ret)
+ dma_addr_t *dma_handle, void **ret, gfp_t flag)
{
struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
if (!mem)
return 0;
- *ret = __dma_alloc_from_coherent(mem, size, dma_handle);
+ *ret = __dma_alloc_from_coherent(mem, size, dma_handle, flag);
return 1;
}
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
+void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle,
+ gfp_t flag)
{
if (!dma_coherent_default_memory)
return NULL;
return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
- dma_handle);
+ dma_handle, flag);
}
static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index b0038ca..bfea1d2 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -272,7 +272,7 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
WARN_ON_ONCE(!dev->coherent_dma_mask);
- if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr))
+ if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr, flag))
return cpu_addr;
/* let the implementation decide on the zone to allocate from: */
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/4] arm64: vdso32: Address various issues
From: Vincenzo Frascino @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190920142738.qlsjwguc6bpnez63@willie-the-truck>
Hi Will,
this patch series is meant to address the various compilation issues you
reported about arm64 vdso32.
Please let me know if there is still something missing.
Thanks,
Vincenzo
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Vincenzo Frascino (4):
arm64: vdso32: Introduce COMPAT_CC_IS_GCC
arm64: vdso32: Detect binutils support for dmb ishld
arm64: vdso32: Fix compilation warning
arm64: Remove gettimeofday.S
arch/arm64/Kconfig | 5 ++++-
arch/arm64/Makefile | 15 ++-------------
arch/arm64/include/asm/memory.h | 5 +++++
arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
arch/arm64/kernel/vdso/gettimeofday.S | 0
arch/arm64/kernel/vdso32/Makefile | 5 ++++-
6 files changed, 16 insertions(+), 16 deletions(-)
delete mode 100644 arch/arm64/kernel/vdso/gettimeofday.S
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/4] arm64: vdso32: Introduce COMPAT_CC_IS_GCC
From: Vincenzo Frascino @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926060353.54894-1-vincenzo.frascino@arm.com>
As reported by Will Deacon the .config file and the generated
include/config/auto.conf can end up out of sync after a set of
commands since CONFIG_CROSS_COMPILE_COMPAT_VDSO is not updated
correctly.
The sequence can be reproduced as follows:
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- defconfig
[...]
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- menuconfig
[set CONFIG_CROSS_COMPILE_COMPAT_VDSO="arm-linux-gnueabihf-"]
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
Which results in:
arch/arm64/Makefile:62: CROSS_COMPILE_COMPAT not defined or empty,
the compat vDSO will not be built
even though the compat vDSO has been built:
$ file arch/arm64/kernel/vdso32/vdso.so
arch/arm64/kernel/vdso32/vdso.so: ELF 32-bit LSB pie executable, ARM,
EABI5 version 1 (SYSV), dynamically linked,
BuildID[sha1]=c67f6c786f2d2d6f86c71f708595594aa25247f6, stripped
A similar case that involves changing the configuration parameter multiple
times can be reconducted to the same family of problems.
The reason behind it comes from the fact that the master Makefile includes
that architecture Makefile twice, once before the syncconfig and one after.
Since the synchronization of the files happens only upon syncconfig, the
architecture Makefile included before this event does not see the change in
configuration.
As a consequence of this it is not possible to handle the cross compiler
definitions inside the architecture Makefile.
Address the problem removing CONFIG_CROSS_COMPILE_COMPAT_VDSO and moving
the detection of the correct compiler into Kconfig via COMPAT_CC_IS_GCC.
As a consequence of this it is not possible anymore to set the compat
cross compiler from menuconfig but it requires to be exported via
command line.
E.g.:
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
CROSS_COMPILE_COMPAT=arm-linux-gnueabihf
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/Kconfig | 5 ++++-
arch/arm64/Makefile | 15 ++-------------
2 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 37c610963eee..afe8c948b493 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -110,7 +110,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select GENERIC_GETTIMEOFDAY
- select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT)
+ select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && COMPAT_CC_IS_GCC)
select HANDLE_DOMAIN_IRQ
select HARDIRQS_SW_RESEND
select HAVE_PCI
@@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET
default 0xeffffff900000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
default 0xffffffffffffffff
+config COMPAT_CC_IS_GCC
+ def_bool $(success,$(CROSS_COMPILE_COMPAT)gcc --version | head -n 1 | grep -q arm)
+
source "arch/arm64/Kconfig.platforms"
menu "Kernel Features"
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 84a3d502c5a5..34f53eb11878 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -54,19 +54,8 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
endif
ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
- CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%)
-
- ifeq ($(CONFIG_CC_IS_CLANG), y)
- $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built)
- else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),)
- $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built)
- else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),)
- $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT)
- else
- export CROSS_COMPILE_COMPAT
- export CONFIG_COMPAT_VDSO := y
- compat_vdso := -DCONFIG_COMPAT_VDSO=1
- endif
+ export CONFIG_COMPAT_VDSO := y
+ compat_vdso := -DCONFIG_COMPAT_VDSO=1
endif
KBUILD_CFLAGS += -mgeneral-regs-only $(lseinstr) $(brokengasinst) \
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/4] arm64: vdso32: Detect binutils support for dmb ishld
From: Vincenzo Frascino @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926060353.54894-1-vincenzo.frascino@arm.com>
As reported by Will Deacon, older versions of binutils that do not
support certain types of memory barriers can cause build failure of the
vdso32 library.
Add a compilation time mechanism that detects if binutils supports those
instructions and configure the kernel accordingly.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/include/asm/vdso/compat_barrier.h | 2 +-
arch/arm64/kernel/vdso32/Makefile | 5 ++++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/vdso/compat_barrier.h b/arch/arm64/include/asm/vdso/compat_barrier.h
index fb60a88b5ed4..3fd8fd6d8fc2 100644
--- a/arch/arm64/include/asm/vdso/compat_barrier.h
+++ b/arch/arm64/include/asm/vdso/compat_barrier.h
@@ -20,7 +20,7 @@
#define dmb(option) __asm__ __volatile__ ("dmb " #option : : : "memory")
-#if __LINUX_ARM_ARCH__ >= 8
+#if __LINUX_ARM_ARCH__ >= 8 && defined(CONFIG_AS_DMB_ISHLD)
#define aarch32_smp_mb() dmb(ish)
#define aarch32_smp_rmb() dmb(ishld)
#define aarch32_smp_wmb() dmb(ishst)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 1fba0776ed40..1a3299d901b1 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -55,6 +55,9 @@ endif
VDSO_CAFLAGS += -fPIC -fno-builtin -fno-stack-protector
VDSO_CAFLAGS += -DDISABLE_BRANCH_PROFILING
+# Check for binutils support for dmb ishld
+dmbinstr := $(call as-instr,dmb ishld,-DCONFIG_AS_DMB_ISHLD=1)
+
# Try to compile for ARMv8. If the compiler is too old and doesn't support it,
# fall back to v7. There is no easy way to check for what architecture the code
# is being compiled, so define a macro specifying that (see arch/arm/Makefile).
@@ -62,7 +65,7 @@ VDSO_CAFLAGS += $(call cc32-option,-march=armv8-a -D__LINUX_ARM_ARCH__=8,\
-march=armv7-a -D__LINUX_ARM_ARCH__=7)
VDSO_CFLAGS := $(VDSO_CAFLAGS)
-VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1
+VDSO_CFLAGS += -DENABLE_COMPAT_VDSO=1 $(dmbinstr)
# KBUILD_CFLAGS from top-level Makefile
VDSO_CFLAGS += -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
-fno-strict-aliasing -fno-common \
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 4/4] arm64: Remove gettimeofday.S
From: Vincenzo Frascino @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926060353.54894-1-vincenzo.frascino@arm.com>
gettimeofday.S was originally removed with the introduction of the
support for Unified vDSOs in arm64 and replaced with the C
implementation.
The file seems again present in the repository due to a side effect of
rebase.
Remove the file again.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/kernel/vdso/gettimeofday.S | 0
1 file changed, 0 insertions(+), 0 deletions(-)
delete mode 100644 arch/arm64/kernel/vdso/gettimeofday.S
diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
deleted file mode 100644
index e69de29bb2d1..000000000000
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 3/4] arm64: vdso32: Fix compilation warning
From: Vincenzo Frascino @ 2019-09-26 6:03 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: ard.biesheuvel, catalin.marinas, ndesaulniers, tglx,
vincenzo.frascino, will
In-Reply-To: <20190926060353.54894-1-vincenzo.frascino@arm.com>
As reported by Will Deacon the following compilation warning appears
during the compilation of the vdso32:
In file included from ./arch/arm64/include/asm/thread_info.h:17:0,
from ./include/linux/thread_info.h:38,
from ./arch/arm64/include/asm/preempt.h:5,
from ./include/linux/preempt.h:78,
from ./include/linux/spinlock.h:51,
from ./include/linux/seqlock.h:36,
from ./include/linux/time.h:6,
from .../work/linux/lib/vdso/gettimeofday.c:7,
from <command-line>:0:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
^
In file included from ./arch/arm64/include/asm/pgtable-hwdef.h:8:0,
from ./arch/arm64/include/asm/processor.h:34,
from ./arch/arm64/include/asm/elf.h:118,
from ./include/linux/elf.h:5,
from ./include/linux/elfnote.h:62,
from arch/arm64/kernel/vdso32/note.c:11:
./arch/arm64/include/asm/memory.h: In function ‘__tag_set’:
./arch/arm64/include/asm/memory.h:233:15: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
^
This happens because few 64 bit compilation headers are included during
the generation of vdso32.
Fix the issue redefining the __tag_set function.
Note: This fix is meant to be temporary, a more comprehensive solution
based on the refactoring of the generic headers will be provided with a
future patch set. At that point it will be possible to revert this patch.
Cc: Will Deacon <will@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
arch/arm64/include/asm/memory.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index b61b50bf68b1..b1c8c43234c5 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -228,11 +228,16 @@ static inline unsigned long kaslr_offset(void)
#define __tag_get(addr) 0
#endif /* CONFIG_KASAN_SW_TAGS */
+#ifdef __aarch64__
static inline const void *__tag_set(const void *addr, u8 tag)
{
u64 __addr = (u64)addr & ~__tag_shifted(0xff);
return (const void *)(__addr | __tag_shifted(tag));
}
+#else
+/* Unused in 32 bit mode */
+#define __tag_set(addr, tag) 0
+#endif
/*
* Physical vs virtual RAM address space conversion. These are
--
2.23.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] soc: ti: ti_sci_pm_domains: Store device id in platform device
From: Tero Kristo @ 2019-09-26 6:20 UTC (permalink / raw)
To: Lokesh Vutla, Nishanth Menon, Santosh Shilimkar
Cc: Sekhar Nori, Linux ARM Mailing List
In-Reply-To: <42e35997-24d5-22d6-28f6-63e4aeac230d@ti.com>
On 26/09/2019 06:33, Lokesh Vutla wrote:
> Hi Tero,
>
> On 24/09/19 10:15 AM, Lokesh Vutla wrote:
>> Hi Tero,
>>
>> On 23/09/19 12:07 PM, Tero Kristo wrote:
>>> On 23/09/2019 06:34, Lokesh Vutla wrote:
>>>> Device ID that is passed from power-domains is used by peripheral
>>>> drivers for communicating with sysfw. Instead of individual drivers
>>>> traversing power-domains entry in DT node, store the device ID in
>>>> platform_device so that drivers can directly access it.
>>>
>>> Uhm, isn't this kind of wrong place to allocate the ID? The power domain
>>
>> I do agree that this might not be a right place, but I couldn't find a better
>> place to populate id. Below is the flow on how platform_device gets created.
>> of_platform_default_populate_init
>> ->of_platform_default_populate
>> -> of_platform_populate
>> ->of_platform_bus_create
>> -> of_platform_device_create_pdata
>> -> of_device_alloc
>> -> platform_device_alloc("", PLATFORM_DEVID_NONE);
>>
>> At this point platform_device gets created with dummy device_id. Also there are
>> no hooks to add custom device_ids.
>>
>>> solution itself is a client also. In theory, someone could access the pdev->id
>>
>> Nope, this is done in dev_pm_domain_attach which is called before driver probe
>> in platform_drv_probe().
>
> If there are no objections, can this patch be picked?
I don't think this patch is still quite right, as it is clearly a hack
(you modify a platform device field outside the chain that actually
allocates it and uses it for some purpose.)
The issue I see here is really easy potential breakage if the pdev->id
is changed to be used something in the OF platform device chain. This
hack would then break as it is completely TI K3 specific, and if any
drivers depend on it, they would break also.
So, IMHO it is a NAK for this patch from my side. It is too hackish, and
there is a way to handle this from drivers currently (yes, the
alternative is bit more painful but it is certain to work going forward
also.)
-Tero
>
> Thanks and regards,
> Lokesh
>
>>
>>> before this. pdev->id should be assigned by bus driver so that it can be
>>> properly handled within platform_device_add.
>>
>> DT doesn't provide any such facility for populating device_add. I am open for
>> any suggestions :)
>>
>> Thanks and regards,
>> Lokesh
>>
>>>
>>> -Tero
>>>
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>>>> ---
>>>> drivers/soc/ti/ti_sci_pm_domains.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/ti/ti_sci_pm_domains.c
>>>> b/drivers/soc/ti/ti_sci_pm_domains.c
>>>> index 8c2a2f23982c..a124ac409124 100644
>>>> --- a/drivers/soc/ti/ti_sci_pm_domains.c
>>>> +++ b/drivers/soc/ti/ti_sci_pm_domains.c
>>>> @@ -116,6 +116,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain
>>>> *domain,
>>>> struct of_phandle_args pd_args;
>>>> struct ti_sci_pm_domain *ti_sci_genpd = genpd_to_ti_sci_pd(domain);
>>>> const struct ti_sci_handle *ti_sci = ti_sci_genpd->ti_sci;
>>>> + struct platform_device *pdev = to_platform_device(dev);
>>>> struct ti_sci_genpd_dev_data *sci_dev_data;
>>>> struct generic_pm_domain_data *genpd_data;
>>>> int idx, ret = 0;
>>>> @@ -129,6 +130,7 @@ static int ti_sci_pd_attach_dev(struct generic_pm_domain
>>>> *domain,
>>>> return -EINVAL;
>>>> idx = pd_args.args[0];
>>>> + pdev->id = idx;
>>>> /*
>>>> * Check the validity of the requested idx, if the index is not valid
>>>>
>>>
>>> --
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [EXT] [PATCH v3] serial: imx: adapt rx buffer and dma periods
From: Ardelean, Alexandru @ 2019-09-26 6:37 UTC (permalink / raw)
To: aford173@gmail.com, philipp.puschmann@emlix.com
Cc: fugang.duan@nxp.com, kernel@pengutronix.de,
linux-serial@vger.kernel.org, gregkh@linuxfoundation.org,
s.hauer@pengutronix.de, jslaby@suse.com,
linux-kernel@vger.kernel.org, linux-imx@nxp.com,
festevam@gmail.com, u.kleine-koenig@pengutronix.de,
yibin.gong@nxp.com, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org, l.stach@pengutronix.de
In-Reply-To: <CAHCN7xL0BKmt8xrhuB4rrvOqkCM5AUJ6YAzbcU8eNDXbzj4fZg@mail.gmail.com>
On Wed, 2019-09-25 at 10:14 -0500, Adam Ford wrote:
> [External]
>
> On Fri, Sep 20, 2019 at 2:06 AM Philipp Puschmann
> <philipp.puschmann@emlix.com> wrote:
> > Hi Andy,
> >
> > Am 20.09.19 um 05:42 schrieb Andy Duan:
> > > From: Philipp Puschmann <philipp.puschmann@emlix.com> Sent: Thursday,
> > > September 19, 2019 10:51 PM
> > > > Using only 4 DMA periods for UART RX is very few if we have a high
> > > > frequency
> > > > of small transfers - like in our case using Bluetooth with many
> > > > small packets
> > > > via UART - causing many dma transfers but in each only filling a
> > > > fraction of a
> > > > single buffer. Such a case may lead to the situation that DMA RX
> > > > transfer is
> > > > triggered but no free buffer is available. When this happens dma
> > > > channel ist
> > > > stopped - with the patch
> > > > "dmaengine: imx-sdma: fix dma freezes" temporarily only - with the
> > > > possible
> > > > consequences that:
>
> I have an i.MX6Q with Wl1837MOD on UART 2 with flow control, and I am
> getting Bluetooth transfer timeouts.
> (see imx6-logicpd-som.dtsi)
>
> On top of 5.3.1, I have installed:
>
> dmaengine: imx-sdma: fix buffer ownership
> dmaengine: imx-sdma: fix dma freezes
> dmaengine: imx-sdma: drop redundant variable
> dmaengine: imx-sdma: fix kernel hangs with SLUB slab allocator
> serial: imx: adapt rx buffer and dma periods
>
> and I still get timeouts:
>
> [ 66.632006] Bluetooth: hci0: command 0xff36 tx timeout
> [ 76.790499] Bluetooth: hci0: command 0x1001 tx timeout
> [ 87.110488] Bluetooth: hci0: command 0xff36 tx timeout
> [ 97.270507] Bluetooth: hci0: command 0x1001 tx timeout
> [ 107.590457] Bluetooth: hci0: command 0xff36 tx timeout
> [ 117.750477] Bluetooth: hci0: command 0x1001 tx timeout
> [ 226.390499] Bluetooth: hci0: command 0xfe38 tx timeout
> [ 231.590735] Bluetooth: hci0: command tx timeout
>
> I did a bisect and found the start of my problems came from
>
> 361deb7243d2 ("dmaengine: dmatest: wrap src & dst data into a struct")
That commit only touches `drivers/dma/dmatest.c`
Are you using that module?
It's a "unit-test" module for testing DMAengine drivers.
The only way that can break anything [from what I can tell], is if it is
being run. It will probably put the DMA into a weird state (it is a test-
module after-all), and it may require some DMAs to be reset.
I admit it would be nice that the test-module would put the DMA back into a
normal-working state, but that effort could be big for some cases.
>
> This happened sometime between v5.0 and v5.1
>
> Is there a patch I missed somewhere? Do I need to change my device
> tree configuration somehow to allocate the proper DMA memory?
>
>
>
> > > > with disabled hw flow control:
> > > > If enough data is incoming on UART port the RX FIFO runs over and
> > > > characters will be lost. What then happens depends on upper
> > > > layer.
> > > >
> > > > with enabled hw flow control:
> > > > If enough data is incoming on UART port the RX FIFO reaches a
> > > > level
> > > > where CTS is deasserted and remote device sending the data stops.
> > > > If it fails to stop timely the i.MX' RX FIFO may run over and
> > > > data
> > > > get lost. Otherwise it's internal TX buffer may getting filled to
> > > > a point where it runs over and data is again lost. It depends on
> > > > the remote device how this case is handled and if it is
> > > > recoverable.
> > > >
> > > > Obviously we want to avoid having no free buffers available. So we
> > > > decrease
> > > > the size of the buffers and increase their number and the total
> > > > buffer size.
> > > >
> > > > Signed-off-by: Philipp Puschmann <philipp.puschmann@emlix.com>
> > > > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> > > > ---
> > > >
> > > > Changelog v3:
> > > > - enhance description
> > > >
> > > > Changelog v2:
> > > > - split this patch from series "Fix UART DMA freezes for iMX6"
> > > > - add Reviewed-by tag
> > > >
> > > > drivers/tty/serial/imx.c | 5 ++---
> > > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > > index
> > > > 87c58f9f6390..51dc19833eab 100644
> > > > --- a/drivers/tty/serial/imx.c
> > > > +++ b/drivers/tty/serial/imx.c
> > > > @@ -1034,8 +1034,6 @@ static void imx_uart_timeout(struct
> > > > timer_list *t)
> > > > }
> > > > }
> > > >
> > > > -#define RX_BUF_SIZE (PAGE_SIZE)
> > > > -
> > > > /*
> > > > * There are two kinds of RX DMA interrupts(such as in the MX6Q):
> > > > * [1] the RX DMA buffer is full.
> > > > @@ -1118,7 +1116,8 @@ static void imx_uart_dma_rx_callback(void
> > > > *data) }
> > > >
> > > > /* RX DMA buffer periods */
> > > > -#define RX_DMA_PERIODS 4
> > > > +#define RX_DMA_PERIODS 16
> > > > +#define RX_BUF_SIZE (PAGE_SIZE / 4)
> > > >
> > > Why to decrease the DMA RX buffer size here ?
> > >
> > > The current DMA implementation support DMA cyclic mode, one SDMA BD
> > > receive one Bluetooth frame can
> > > bring better performance.
> > > As you know, for L2CAP, a maximum transmission unit (MTU) associated
> > > with the largest Baseband payload
> > > is 341 bytes for DH5 packets.
> > >
> > > So I suggest to increase RX_BUF_SIZE along with RX_DMA_PERIODS to
> > > feasible value.
> >
> > I debugged and developed this patches on a system with a 4.15 kernel.
> > When prepared for upstream i have adapted
> > some details and missed a important thing here. It should say:
> >
> > +#define RX_BUF_SIZE (RX_DMA_PERIODS * PAGE_SIZE / 4)
> >
> > Yes, i wanted to increase the total buffer size too, even wrote it in
> > the description.
> > I will prepare a version 4, thanks for the hint.
> >
> > Just for info: A single RX DMA period aka buffer can be filled with
> > mutliple packets in regard of the upper layer, here BT.
> >
> >
> > Regards,
> > Philipp
> > > Andy
> > >
> > > > static int imx_uart_start_rx_dma(struct imx_port *sport) {
> > > > --
> > > > 2.23.0
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH] ARM/hw_breakpoint: add ARMv8.1/ARMv8.2 debug architecutre versions support in enable_monitor_mode()
From: Candle Sun @ 2019-09-26 7:38 UTC (permalink / raw)
To: will, mark.rutland, linux
Cc: Nianfu Bai, Candle Sun, linux-kernel, linux-arm-kernel
From: Candle Sun <candle.sun@unisoc.com>
When ARMv8.1/ARMv8.2 cores are used in AArch32 mode,
arch_hw_breakpoint_init() in arch/arm/kernel/hw_breakpoint.c will be used.
From ARMv8 specification, different debug architecture versions defined:
* 0110 ARMv8, v8 Debug architecture.
* 0111 ARMv8.1, v8 Debug architecture, with Virtualization Host Extensions.
* 1000 ARMv8.2, v8.2 Debug architecture.
So missing ARMv8.1/ARMv8.2 cases will cause enable_monitor_mode() function
returns -ENODEV, and arch_hw_breakpoint_init() will fail.
Signed-off-by: Candle Sun <candle.sun@unisoc.com>
Signed-off-by: Nianfu Bai <nianfu.bai@unisoc.com>
---
arch/arm/include/asm/hw_breakpoint.h | 2 ++
arch/arm/kernel/hw_breakpoint.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm/include/asm/hw_breakpoint.h b/arch/arm/include/asm/hw_breakpoint.h
index ac54c06..9137ef6 100644
--- a/arch/arm/include/asm/hw_breakpoint.h
+++ b/arch/arm/include/asm/hw_breakpoint.h
@@ -53,6 +53,8 @@ static inline void decode_ctrl_reg(u32 reg,
#define ARM_DEBUG_ARCH_V7_MM 4
#define ARM_DEBUG_ARCH_V7_1 5
#define ARM_DEBUG_ARCH_V8 6
+#define ARM_DEBUG_ARCH_V8_1 7
+#define ARM_DEBUG_ARCH_V8_2 8
/* Breakpoint */
#define ARM_BREAKPOINT_EXECUTE 0
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index b0c195e..cb99612 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -246,6 +246,8 @@ static int enable_monitor_mode(void)
case ARM_DEBUG_ARCH_V7_ECP14:
case ARM_DEBUG_ARCH_V7_1:
case ARM_DEBUG_ARCH_V8:
+ case ARM_DEBUG_ARCH_V8_1:
+ case ARM_DEBUG_ARCH_V8_2:
ARM_DBG_WRITE(c0, c2, 2, (dscr | ARM_DSCR_MDBGEN));
isb();
break;
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] arm64: Allow disabling of the compat vDSO
From: Catalin Marinas @ 2019-09-26 7:47 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: Ard Biesheuvel, Nick Desaulniers, LKML, Thomas Gleixner,
Will Deacon, Linux ARM
In-Reply-To: <a7e06b86-facd-21de-c47c-246d0da8d80d@arm.com>
On Thu, Sep 26, 2019 at 01:06:50AM +0100, Vincenzo Frascino wrote:
> On 9/25/19 6:08 PM, Catalin Marinas wrote:
> > On Wed, Sep 25, 2019 at 09:53:16AM -0700, Nick Desaulniers wrote:
> >> On Wed, Sep 25, 2019 at 6:09 AM Catalin Marinas <catalin.marinas@arm.com> wrote:
> >>> Suggestions for future improvements of the compat vDSO handling:
> >>>
> >>> - replace the CROSS_COMPILE_COMPAT prefix with a full COMPATCC; maybe
> >>> check that it indeed produces 32-bit code
>
> CROSS_COMPILE_COMPAT is called like this for symmetry with CROSS_COMPILE.
Actually, what gets in the way is the CONFIG_CROSS_COMPILE_COMPAT_VDSO.
We can keep CROSS_COMPILE_COMPAT together with COMPATCC initialised to
$(CROSS_COMPILE_COMPAT)gcc. When we will be able to build the compat
vDSO with clang, we just pass COMPATCC=clang on the make line and the
kernel Makefile will figure out the --target option from
CROSS_COMPILE_COMPAT (see how CLANG_FLAGS is handled).
If we stick only to env variables or make cmd line (without Kconfig) for
the compiler name, we can add a COMPATCC_IS_CLANG in the Kconfig
directly and simply not allow the enabling the COMPAT_VDSO if we don't
have the right compiler. This saves us warnings during build.
> >>> - check whether COMPATCC is clang or not rather than CC_IS_CLANG, which
> >>> only checks the native compiler
> >>
> >> When cross compiling, IIUC CC_IS_CLANG is referring to CC which is the
> >> cross compiler, which is different than HOSTCC which is the host
> >> compiler. HOSTCC is used mostly for things in scripts/ while CC is
> >> used to compile a majority of the kernel in a cross compile.
> >
> > We need the third compiler here for the compat vDSO (at least with gcc),
> > COMPATCC. I'm tempted to just drop the CONFIG_CROSS_COMPILE_COMPAT_VDSO
> > altogether and only rely on a COMPATCC. This way we can add
> > COMPATCC_IS_CLANG etc. in the Kconfig checks directly.
> >
> > If clang can build both 32 and 64-bit with the same binary (just
> > different options), we could maybe have COMPATCC default to CC and add a
> > check on whether COMPATCC generates 32-bit binaries.
>
> clang requires the --target option for specifying the 32bit triple.
> Basically $(TRIPLE)-gcc is equivalent to gcc --target $(TRIPLE).
> We need a configuration option to encode this.
Since we don't have a CONFIG_* option for the cross-compiler prefix, we
shouldn't have one for the compat compiler either. If you want to build
the compat vDSO with clang, just pass COMPATCC=clang together with
CROSS_COMPILE_COMPAT. We can add Kconfig checks to actually verify that
COMPATCC generates 32-bit binaries (e.g. COMPATCC_CAN_LINK32).
> >>> - clean up the headers includes; vDSO should not include kernel-only
> >>> headers that may even contain code patched at run-time
> >>
> >> This is a big one; Clang validates the inline asm constraints for
> >> extended inline assembly, GCC does not for dead code. So Clang chokes
> >> on the inclusion of arm64 headers using extended inline assembly when
> >> being compiled for arm-linux-gnueabi.
> >
> > Whether clang or gcc, I'd like this fixed anyway. At some point we may
> > inadvertently rely on some code which is patched at boot time for the
> > kernel code but not for the vDSO.
>
> Do we have any code of this kind in header files?
>
> The vDSO library uses only a subset of the headers (mainly Macros) hence all the
> unused symbols should be compiled out. Is your concern only theoretical or do
> you have an example on where this could be happening?
At the moment it's rather theoretical.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs
From: Marco Felsch @ 2019-09-26 7:59 UTC (permalink / raw)
To: Anson Huang
Cc: aisheng.dong, shawnguo, s.hauer, linux-kernel, Linux-imx, kernel,
leonard.crestez, festevam, linux-arm-kernel
In-Reply-To: <1569406066-16626-1-git-send-email-Anson.Huang@nxp.com>
Hi,
On 19-09-25 18:07, Anson Huang wrote:
> The SCU firmware does NOT always have return value stored in message
> header's function element even the API has response data, those special
> APIs are defined as void function in SCU firmware, so they should be
> treated as return success always.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> - This patch is based on the patch of https://patchwork.kernel.org/patch/11129553/
> ---
> drivers/firmware/imx/imx-scu.c | 34 ++++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/imx/imx-scu.c b/drivers/firmware/imx/imx-scu.c
> index 869be7a..ced5b12 100644
> --- a/drivers/firmware/imx/imx-scu.c
> +++ b/drivers/firmware/imx/imx-scu.c
> @@ -78,6 +78,11 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] = {
> -EIO, /* IMX_SC_ERR_FAIL */
> };
>
> +static const struct imx_sc_rpc_msg whitelist[] = {
> + { .svc = IMX_SC_RPC_SVC_MISC, .func = IMX_SC_MISC_FUNC_UNIQUE_ID },
> + { .svc = IMX_SC_RPC_SVC_MISC, .func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS },
> +};
Is this going to be extended in the near future? I see some upcoming
problems here if someone uses a different scu-fw<->kernel combination as
nxp would suggest.
Regards,
Marco
> +
> static struct imx_sc_ipc *imx_sc_ipc_handle;
>
> static inline int imx_sc_to_linux_errno(int errno)
> @@ -157,11 +162,24 @@ static int imx_scu_ipc_write(struct imx_sc_ipc *sc_ipc, void *msg)
> return 0;
> }
>
> +static bool imx_scu_call_skip_return_value_check(uint8_t svc, uint8_t func)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(whitelist); i++)
> + if (svc == whitelist[i].svc &&
> + func == whitelist[i].func)
> + return true;
> +
> + return false;
> +}
> +
> /*
> * RPC command/response
> */
> int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp)
> {
> + uint8_t saved_svc, saved_func;
> struct imx_sc_rpc_msg *hdr;
> int ret;
>
> @@ -171,8 +189,11 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp)
> mutex_lock(&sc_ipc->lock);
> reinit_completion(&sc_ipc->done);
>
> - if (have_resp)
> + if (have_resp) {
> sc_ipc->msg = msg;
> + saved_svc = ((struct imx_sc_rpc_msg *)msg)->svc;
> + saved_func = ((struct imx_sc_rpc_msg *)msg)->func;
> + }
> sc_ipc->count = 0;
> ret = imx_scu_ipc_write(sc_ipc, msg);
> if (ret < 0) {
> @@ -190,7 +211,16 @@ int imx_scu_call_rpc(struct imx_sc_ipc *sc_ipc, void *msg, bool have_resp)
>
> /* response status is stored in hdr->func field */
> hdr = msg;
> - ret = hdr->func;
> + /*
> + * Some special SCU firmware APIs do NOT have return value
> + * in hdr->func, but they do have response data, those special
> + * APIs are defined as void function in SCU firmware, so they
> + * should be treated as return success always.
> + */
> + if (!imx_scu_call_skip_return_value_check(saved_svc, saved_func))
> + ret = hdr->func;
> + else
> + ret = 0;
> }
>
> out:
> --
> 2.7.4
>
>
>
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH] firmware: imx: Skip return value check for some special SCU firmware APIs
From: Anson Huang @ 2019-09-26 8:03 UTC (permalink / raw)
To: Marco Felsch
Cc: Aisheng Dong, shawnguo@kernel.org, s.hauer@pengutronix.de,
linux-kernel@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
Leonard Crestez, festevam@gmail.com,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190926075914.i7tsd3cbpitrqe4q@pengutronix.de>
Hi, Marco
> On 19-09-25 18:07, Anson Huang wrote:
> > The SCU firmware does NOT always have return value stored in message
> > header's function element even the API has response data, those
> > special APIs are defined as void function in SCU firmware, so they
> > should be treated as return success always.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > - This patch is based on the patch of
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fpatch%2F11129553%2F&data=02%7C01%7Canson.
> huang%
> >
> 40nxp.com%7C1f4108cc25eb4618f43c08d742576fa3%7C686ea1d3bc2b4c6fa
> 92cd99
> >
> c5c301635%7C0%7C0%7C637050815608963707&sdata=BZBg4cOR2rP%2
> BRBNn15i
> > Qq3%2FXBYwhuCLkgYzFRbfEgVU%3D&reserved=0
> > ---
> > drivers/firmware/imx/imx-scu.c | 34
> > ++++++++++++++++++++++++++++++++--
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/imx/imx-scu.c
> > b/drivers/firmware/imx/imx-scu.c index 869be7a..ced5b12 100644
> > --- a/drivers/firmware/imx/imx-scu.c
> > +++ b/drivers/firmware/imx/imx-scu.c
> > @@ -78,6 +78,11 @@ static int imx_sc_linux_errmap[IMX_SC_ERR_LAST] =
> {
> > -EIO, /* IMX_SC_ERR_FAIL */
> > };
> >
> > +static const struct imx_sc_rpc_msg whitelist[] = {
> > + { .svc = IMX_SC_RPC_SVC_MISC, .func =
> IMX_SC_MISC_FUNC_UNIQUE_ID },
> > + { .svc = IMX_SC_RPC_SVC_MISC, .func =
> > +IMX_SC_MISC_FUNC_GET_BUTTON_STATUS }, };
>
> Is this going to be extended in the near future? I see some upcoming
> problems here if someone uses a different scu-fw<->kernel combination as
> nxp would suggest.
Could be, but I checked the current APIs, ONLY these 2 will be used in Linux kernel, so
I ONLY add these 2 APIs for now.
However, after rethink, maybe we should add another imx_sc_rpc API for those special
APIs? To avoid checking it for all the APIs called which may impact some performance.
Still under discussion, if you have better idea, please advise, thanks!
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/4] arm64: vdso32: Introduce COMPAT_CC_IS_GCC
From: Catalin Marinas @ 2019-09-26 8:06 UTC (permalink / raw)
To: Vincenzo Frascino
Cc: ard.biesheuvel, ndesaulniers, linux-kernel, tglx, will,
linux-arm-kernel
In-Reply-To: <20190926060353.54894-2-vincenzo.frascino@arm.com>
On Thu, Sep 26, 2019 at 07:03:50AM +0100, Vincenzo Frascino wrote:
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 37c610963eee..afe8c948b493 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -110,7 +110,7 @@ config ARM64
> select GENERIC_STRNLEN_USER
> select GENERIC_TIME_VSYSCALL
> select GENERIC_GETTIMEOFDAY
> - select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT)
> + select GENERIC_COMPAT_VDSO if (!CPU_BIG_ENDIAN && COMPAT && COMPAT_CC_IS_GCC)
> select HANDLE_DOMAIN_IRQ
> select HARDIRQS_SW_RESEND
> select HAVE_PCI
> @@ -313,6 +313,9 @@ config KASAN_SHADOW_OFFSET
> default 0xeffffff900000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
> default 0xffffffffffffffff
>
> +config COMPAT_CC_IS_GCC
> + def_bool $(success,$(CROSS_COMPILE_COMPAT)gcc --version | head -n 1 | grep -q arm)
Nitpick: I prefer COMPATCC instead of COMPAT_CC for consistency with
HOSTCC.
Now, could we not generate a COMPATCC in the Makefile and use
$(COMPATCC) here instead of $(CROSS_COMPILE_COMPAT)gcc? It really
doesn't make sense to check that gcc is gcc.
A next step would be to check that COMPATCC can actually generate 32-bit
objects. But it's not essential at this stage.
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 84a3d502c5a5..34f53eb11878 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -54,19 +54,8 @@ $(warning Detected assembler with broken .inst; disassembly will be unreliable)
> endif
>
> ifeq ($(CONFIG_GENERIC_COMPAT_VDSO), y)
> - CROSS_COMPILE_COMPAT ?= $(CONFIG_CROSS_COMPILE_COMPAT_VDSO:"%"=%)
> -
> - ifeq ($(CONFIG_CC_IS_CLANG), y)
> - $(warning CROSS_COMPILE_COMPAT is clang, the compat vDSO will not be built)
> - else ifeq ($(strip $(CROSS_COMPILE_COMPAT)),)
> - $(warning CROSS_COMPILE_COMPAT not defined or empty, the compat vDSO will not be built)
> - else ifeq ($(shell which $(CROSS_COMPILE_COMPAT)gcc 2> /dev/null),)
> - $(error $(CROSS_COMPILE_COMPAT)gcc not found, check CROSS_COMPILE_COMPAT)
> - else
> - export CROSS_COMPILE_COMPAT
> - export CONFIG_COMPAT_VDSO := y
> - compat_vdso := -DCONFIG_COMPAT_VDSO=1
> - endif
> + export CONFIG_COMPAT_VDSO := y
> + compat_vdso := -DCONFIG_COMPAT_VDSO=1
> endif
Has CONFIG_CROSS_COMPILE_COMPAT_VDSO actually been removed from
lib/vdso/Kconfig? (I haven't checked the subsequent patches).
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox