* [PATCH 1/2] pwm: pwm-mxs: use devm_platform_ioremap_resource() to simplify code
From: Anson Huang @ 2019-09-24 9:42 UTC (permalink / raw)
To: thierry.reding, shawnguo, s.hauer, kernel, festevam, linux-pwm,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
Use the new helper devm_platform_ioremap_resource() which wraps the
platform_get_resource() and devm_ioremap_resource() together, to
simplify the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
- This is a resend version of patch: https://patchwork.kernel.org/patch/11048365/
---
drivers/pwm/pwm-mxs.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-mxs.c b/drivers/pwm/pwm-mxs.c
index 04c0f6b..b14376b 100644
--- a/drivers/pwm/pwm-mxs.c
+++ b/drivers/pwm/pwm-mxs.c
@@ -126,15 +126,13 @@ static int mxs_pwm_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct mxs_pwm_chip *mxs;
- struct resource *res;
int ret;
mxs = devm_kzalloc(&pdev->dev, sizeof(*mxs), GFP_KERNEL);
if (!mxs)
return -ENOMEM;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- mxs->base = devm_ioremap_resource(&pdev->dev, res);
+ mxs->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mxs->base))
return PTR_ERR(mxs->base);
--
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 V7 0/2] mailbox: arm: introduce smc triggered mailbox
From: Peng Fan @ 2019-09-24 9:39 UTC (permalink / raw)
To: André Przywara, robh+dt@kernel.org, mark.rutland@arm.com,
jassisinghbrar@gmail.com, sudeep.holla@arm.com,
f.fainelli@gmail.com
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, dl-linux-imx
In-Reply-To: <30b20102-40d5-1446-1513-e64b47459db8@arm.com>
Hi Andre,
> Subject: Re: [PATCH V7 0/2] mailbox: arm: introduce smc triggered mailbox
>
> On 23/09/2019 07:36, Peng Fan wrote:
>
> Hi Peng,
>
> thanks for the update!
>
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > V7:
> > Typo fix
> > #mbox-cells changed to 0
> > Add a new header file arm-smccc-mbox.h Use ARM_SMCCC_IS_64
> >
> > Andre,
> > The function_id is still kept in arm_smccc_mbox_cmd, because
> > arm,func-id property is optional, so clients could pass function_id to mbox
> driver.
>
> Well, to be honest, this is the main thing I am opposing:
>
> It should *not* be optional.
>
> The controller driver DT node should *always* contain the function ID.
> The reasons for that I explained in the other emails to Jassi:
> We can't safely execute smc calls from the Linux kernel, unless we also
> comply with the SMCCC standard. So we should not leave the choice of the
> function ID to the mailbox client.
> Also this much better separates the mailbox controller driver from the client.
>
> So I think we should reach an agreement here.
I am ok to mark it as a required property in v9, but I am not sure Jassi accept it.
Thanks,
Peng.
>
> Cheers,
> Andre
>
> > V6:
> > Switch to per-channel a mbox controller Drop arm,num-chans,
> > transports, method Add arm,hvc-mbox compatible Fix smc/hvc args, drop
> > client id and use correct type.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fcover%2F11146641%2F&data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637048990507177346&sdata=N%2BWdEXh9QXn
> F4cn2DjCwHk
> > qpD1JxtP%2BBfQwn3E3lzYs%3D&reserved=0
> >
> > V5:
> > yaml fix
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fcover%2F11117741%2F&data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637048990507177346&sdata=XcRM%2FH4ZQAU
> QmT%2FDlGzq
> > 93LjlVhUYuvEmdYSezivTog%3D&reserved=0
> >
> > V4:
> > yaml fix for num-chans in patch 1/2.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fcover%2F11116521%2F&data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637048990507177346&sdata=8f0Etfnd29Q6LHx
> bymLVyvEE
> > 7ElGZzN3uVMcIL%2BmZNQ%3D&reserved=0
> >
> > V3:
> > Drop interrupt
> > Introduce transports for mem/reg usage Add chan-id for mem usage
> > Convert to yaml format
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fcover%2F11043541%2F&data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637048990507177346&sdata=U2%2BjaU06Iu0h
> ULuCz0RT1r
> > vhSfjsu%2BWuKzjTfY7MuNw%3D&reserved=0
> >
> > V2:
> > This is a modified version from Andre Przywara's patch series
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
> ng.fan%40nxp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637048990507177346&sda
> ta=dku%2BfT%2BDLilVU5T0fdHMzuUi4DYCM7aV1FQB4vaKCVE%3D&re
> served=0.
> > The modification are mostly:
> > Introduce arm,num-chans
> > Introduce arm_smccc_mbox_cmd
> > txdone_poll and txdone_irq are both set to false arm,func-ids are
> > kept, but as an optional property.
> > Rewords SCPI to SCMI, because I am trying SCMI over SMC, not SCPI.
> > Introduce interrupts notification.
> >
> > [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> > use smc as mailbox, power/clk is included, but only part of clk has
> > been implemented to work with hardware, power domain only supports get
> > name for now.
> >
> > The traditional Linux mailbox mechanism uses some kind of dedicated
> > hardware IP to signal a condition to some other processing unit,
> > typically a dedicated management processor.
> > This mailbox feature is used for instance by the SCMI protocol to
> > signal a request for some action to be taken by the management processor.
> > However some SoCs does not have a dedicated management core to
> provide
> > those services. In order to service TEE and to avoid linux shutdown
> > power and clock that used by TEE, need let firmware to handle power
> > and clock, the firmware here is ARM Trusted Firmware that could also
> > run SCMI service.
> >
> > The existing SCMI implementation uses a rather flexible shared memory
> > region to communicate commands and their parameters, it still requires
> > a mailbox to actually trigger the action.
> >
> > This patch series provides a Linux mailbox compatible service which
> > uses smc calls to invoke firmware code, for instance taking care of SCMI
> requests.
> > The actual requests are still communicated using the standard SCMI way
> > of shared memory regions, but a dedicated mailbox hardware IP can be
> > replaced via this new driver.
> >
> > This simple driver uses the architected SMC calling convention to
> > trigger firmware services, also allows for using "HVC" calls to call
> > into hypervisors or firmware layers running in the EL2 exception level.
> >
> > Patch 1 contains the device tree binding documentation, patch 2
> > introduces the actual mailbox driver.
> >
> > Please note that this driver just provides a generic mailbox
> > mechanism, It could support synchronous TX/RX, or synchronous TX with
> > asynchronous RX. And while providing SCMI services was the reason for
> > this exercise, this driver is in no way bound to this use case, but
> > can be used generically where the OS wants to signal a mailbox
> > condition to firmware or a hypervisor.
> > Also the driver is in no way meant to replace any existing firmware
> > interface, but actually to complement existing interfaces.
> >
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&data=02
> %7C01%7
> >
> Cpeng.fan%40nxp.com%7C52d7e331c222426852a508d740ae7d42%7C686ea
> 1d3bc2b4
> >
> c6fa92cd99c5c301635%7C0%7C0%7C637048990507177346&sdata=fFE
> KK8jYap5
> > igpH6ESGTHYfnhBZMsL2zB%2BdykMdOPwg%3D&reserved=0
> >
> >
> >
> > Peng Fan (2):
> > dt-bindings: mailbox: add binding doc for the ARM SMC/HVC mailbox
> > mailbox: introduce ARM SMC based mailbox
> >
> > .../devicetree/bindings/mailbox/arm-smc.yaml | 95
> ++++++++++++
> > drivers/mailbox/Kconfig | 7 +
> > drivers/mailbox/Makefile | 2 +
> > drivers/mailbox/arm-smc-mailbox.c | 168
> +++++++++++++++++++++
> > 4 files changed, 272 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/mailbox/arm-smc.yaml
> > create mode 100644 drivers/mailbox/arm-smc-mailbox.c
> >
_______________________________________________
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 v7 5/6] PM / devfreq: Add PM QoS support
From: Leonard Crestez @ 2019-09-24 9:27 UTC (permalink / raw)
To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Alexandre Bailon,
Martin Kepplinger, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <3eb36ce6-a9df-7f9a-1492-8747f2648e78@samsung.com>
On 2019-09-24 9:47 AM, Chanwoo Choi wrote:
> Hi,
>
> On 19. 9. 24. 오전 6:10, 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>
>> ---
>> drivers/devfreq/devfreq.c | 76 +++++++++++++++++++++++++++++++++++++++
>> include/linux/devfreq.h | 5 +++
>> 2 files changed, 81 insertions(+)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 7f152a582e78..9887408f23bb 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -22,17 +22,20 @@
>> #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 HZ_PER_KHZ 1000
>
> You have to remove it on patch4.
Done. This was added twice, probably messed up during rebasing.
>> /*
>> * devfreq core provides delayed work based load monitoring helper
>> * functions. Governors can use these or can implement their own
>> @@ -123,10 +126,16 @@ static void devfreq_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 */
>
> I think that it is not necessary. But, if you think it is required,
> Please add the detailed comment with consistent comment style.
Not sure what comment style you mean, it seems like a reasonable
single-line comment.
>> + *min_freq = max(*min_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
>> + devfreq->dev.parent, DEV_PM_QOS_MIN_FREQUENCY));
>> + *max_freq = min(*max_freq, HZ_PER_KHZ * (unsigned long)dev_pm_qos_read_value(
>> + devfreq->dev.parent, DEV_PM_QOS_MAX_FREQUENCY));
>
> If you use the separate variable for getting the value from dev_pm_qos_read_value(),
> you can add this line under 80 char. If there are any special reason,
> I prefer to keep the line under 80 char.
OK
>> /* constraints from sysfs */
>> *min_freq = max(*min_freq, devfreq->min_freq);
>> *max_freq = min(*max_freq, devfreq->max_freq);
>>
>> /* constraints from OPP interface */
>> @@ -605,10 +614,53 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
>> mutex_unlock(&devfreq->lock);
>>
>> return ret;
>> }
>>
>> +/**
>> + * devfreq_qos_notifier_call() - Common handler for QoS constraints.
>> + * @devfreq: the devfreq instance.
>> + */
>> +static int devfreq_qos_notifier_call(struct devfreq *devfreq)
>
> Also, as I commented on patch4, we better to remove 'devfreq' prefix
> for internal function as following:
> devfreq_qos_notifier_call -> qos_notifier_call
OK
>> +{
>> + int err;
>> +
>> + mutex_lock(&devfreq->lock);
>> + err = update_devfreq(devfreq);
>> + mutex_unlock(&devfreq->lock);
>> + if (err)
>> + dev_err(&devfreq->dev, "dvfs for QoS constraints"
>> + " failed with (%d) error\n", err);
>
> 'dvfs' is not full name. Also, the capital letter is more correct.
> But, the devfreq used 'failed to ...' comment style on some points.
>
> I suggest the comment as following:
>
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -628,8 +628,8 @@ static int devfreq_qos_notifier_call(struct devfreq *devfreq)
> err = update_devfreq(devfreq);
> mutex_unlock(&devfreq->lock);
> if (err)
> - dev_err(&devfreq->dev, "dvfs for QoS constraints"
> - " failed with (%d) error\n", err);
> + dev_err(&devfreq->dev,
> + "failed to update frequency with PMQoS (%d)\n", err);
I tried to mirror comment in monitor_devfreq, I'll adjust. The dev_name
of devfreq->dev is usually something unhelpful like "devfreq1" so maybe
&devfreq->dev.parent should be passed instead.
>
>> +
>> + /* QoS is best effort - let all notifiers run on error */
>
> Please remove it.
OK but an explanation as to why errors are not propagated seems worth
keeping.
>> + return NOTIFY_OK;
>> +}
>> +
>> +/**
>> + * devfreq_qos_min_notifier_call() - Callback for QoS min_freq changes.
>> + * @nb: Should be devfreq->nb_min
>> + */
>> +static int devfreq_qos_min_notifier_call(struct notifier_block *nb,
>
> ditto.
> devfreq_qos_min_notifier_call -> qos_min_notifier_call
>
>> + unsigned long val, void *ptr)
>> +{
>> + struct devfreq *devfreq = container_of(nb, struct devfreq, nb_min);
>> +
>
> nitpick. You can remove this line.
OK, if the devfreq_ prefix is removed this can be under 80 chars.
>> @@ -619,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);
>
> Even if devfreq_dev_release() is called at the end of device driver,
> dev_pm_remove_notifier() have returned the value. Need to check
> the return value for checking the error state.
And if dev_pm_qos_remove_notifier returns an error what next?
In cleanup functions there is not much you can do on error other than to
continue cleaning up. This is why they usually return void.
>> if (devfreq->profile->exit)
>> devfreq->profile->exit(devfreq->dev.parent);
>>
>> kfree(devfreq->time_in_state);
>> kfree(devfreq->trans_table);
>> @@ -732,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).> + */
>
> In the devfreq_add_device(), each step has not contained
> the detailed comment. If possible, in order to keep the existing style,
> please remove it.
I can remove it but the comment explains why this is the right step at
which notifiers should be registered so it seems useful.
>> + devfreq->nb_min.notifier_call = devfreq_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 = devfreq_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",
>> @@ -759,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 2bae9ed3c783..8b92ccbd1962 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;
>>
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Suzuki K Poulose @ 2019-09-24 9:13 UTC (permalink / raw)
To: Mark Brown, Catalin Marinas; +Cc: Will Deacon, linux-arm-kernel
In-Reply-To: <20190816121005.GB4039@sirena.co.uk>
On 16/08/2019 13:10, Mark Brown wrote:
> On Fri, Aug 16, 2019 at 11:24:24AM +0100, Catalin Marinas wrote:
>> On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
>
>>>> + if (IS_ENABLED(CONFIG_ARM64_E0PD)) {
>>>> + ftr = read_sysreg_s(SYS_ID_AA64MMFR2_EL1);
>>>> + if ((ftr >> ID_AA64MMFR2_E0PD_SHIFT) & 0xf)
>>>> + return false;
>>>> + }
>
>> What I don't particularly like here is that on big.LITTLE this hunk may
>> have a different behaviour depending on which CPU you run it on. In
>> general, such CPUID access should only be done in a non-preemptible
>> context.
>
>> We probably get away with this during early boot (before CPU caps have
>> been set up) when arm64_kernel_unmapped_at_el0() is false since we only
>> have a single CPU running. Later on at run-time, we either have
>> arm64_kernel_unmapped_at_el0() true, meaning that some CPU is missing
>> E0PD with kaslr_offset() > 0, or the kernel is mapped at EL0 with all
>> CPUs having E0PD. But I find it hard to reason about.
>
> Yes, all this stuff is unfortunately hard to reason about since there's
> several environment changes during boot which have a material effect and
> also multiple different things that might trigger KPTI. IIRC my thinking
> here was that if we turned on KPTI we're turning it on for all CPUs so
> by the time we could be prempted we'd be returning true from the earlier
> check for arm64_kernel_unmapped_at_el0() but it's possible I missed some
> case there. I was trying to avoid disturbing the existing code too much
> unless I had a strong reason to on the basis that I might be missing
> something about the way it was done.
>
>> Could we move the above hunk in this block:
>
>> } else if (!static_branch_likely(&arm64_const_caps_ready)) {
>> ...
>> }
>
>> and reshuffle the rest of the function to only rely on
>> arm64_kernel_unmapped_at_el0() when the caps are ready (at run-time)?
>
> I've added the check, will look at the reshuffle.
>
>>> Thinking about this further, I think we can simply move all of the
>>> 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
>>> arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
>>> unmap_kernel_at_el0()) into a helper function which does the check for
>>> E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
>
>> I agree, this needs some refactoring as we have this decision in three
>> separate places.
>
>> Trying to put my thoughts together. At run-time, with capabilities fully
>> enabled, we want:
>
>> arm64_kernel_use_ng_mappings() == arm64_kernel_unmapped_at_el0()
>
>> KPTI is equivalent to arm64_kernel_unmapped_at_el0()
>
> Yes, this bit is simple - once we're up and running everything is clear.
>
>> I think kaslr_requires_kpti() should access the raw CPUID registers (for
>> E0PD, TX1 bug) and be called only by unmap_kernel_at_el0() and
>> arm64_kernel_use_ng_mappings(), the latter if !arm64_const_caps_ready.
>> The boot CPU should store kaslr_requires_kpti() value somewhere and
>> kpti_install_ng_mappings() should check this variable before deciding to
>> skip the page table rewrite.
>
> We definitely need some variable I think, and I think you're right that
> making the decision on the boot CPU would simplify things a lot. The
> systems with very large memories that are most affected by the cost of
> moving from global to non-global mappings are most likely symmetric
> anyway so only looking at the boot CPU should be fine for that.
>
With KASLR, we already rewrite the page table from __primary_switch() after
relocating the kernel. So, we may be able to perform "raw cpuid check" on
the boot CPU with MMU turned on, before we re-write the pagetables for KASLR
displacement and nG if that is needed (by maybe updating SWWAPPER_MMU_FLAGS) for
the boot CPU and store this information somewhere. Thus we may be able to
avoid another re-write of the pagetables after we have booted the secondaries.
We could continue to do the per-CPU check to see if we need nG mappings
and perform the transition later if needed, like we do now.
Discussing this with Catalin, he suggests to use a variable for the status
of "nG" flag for PTE/PMD_MAYBE_NG, to avoid calling the helper function
all the time. By using the per-CPU check we can make sure the flag is uptodate.
Also, we can continue to fail the hotplugged CPUs if we detect that the
pagetables are Global and the new CPU requires nG (for heterogeneous systems).
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] input: keyboard: imx_keypad: Use 'dev' instead of dereferencing it repeatedly
From: Anson Huang @ 2019-09-24 9:04 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam, swboyd,
mojha, linux-input, linux-arm-kernel, linux-kernel
Cc: Linux-imx
Add helper variable dev = &pdev->dev to simply the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/input/keyboard/imx_keypad.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 5a46d11..3024d2a 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -418,14 +418,15 @@ MODULE_DEVICE_TABLE(of, imx_keypad_of_match);
static int imx_keypad_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
const struct matrix_keymap_data *keymap_data =
- dev_get_platdata(&pdev->dev);
+ dev_get_platdata(dev);
struct imx_keypad *keypad;
struct input_dev *input_dev;
int irq, error, i, row, col;
- if (!keymap_data && !pdev->dev.of_node) {
- dev_err(&pdev->dev, "no keymap defined\n");
+ if (!keymap_data && !dev->of_node) {
+ dev_err(dev, "no keymap defined\n");
return -EINVAL;
}
@@ -433,15 +434,15 @@ static int imx_keypad_probe(struct platform_device *pdev)
if (irq < 0)
return irq;
- input_dev = devm_input_allocate_device(&pdev->dev);
+ input_dev = devm_input_allocate_device(dev);
if (!input_dev) {
- dev_err(&pdev->dev, "failed to allocate the input device\n");
+ dev_err(dev, "failed to allocate the input device\n");
return -ENOMEM;
}
- keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
+ keypad = devm_kzalloc(dev, sizeof(*keypad), GFP_KERNEL);
if (!keypad) {
- dev_err(&pdev->dev, "not enough memory for driver data\n");
+ dev_err(dev, "not enough memory for driver data\n");
return -ENOMEM;
}
@@ -456,16 +457,16 @@ static int imx_keypad_probe(struct platform_device *pdev)
if (IS_ERR(keypad->mmio_base))
return PTR_ERR(keypad->mmio_base);
- keypad->clk = devm_clk_get(&pdev->dev, NULL);
+ keypad->clk = devm_clk_get(dev, NULL);
if (IS_ERR(keypad->clk)) {
- dev_err(&pdev->dev, "failed to get keypad clock\n");
+ dev_err(dev, "failed to get keypad clock\n");
return PTR_ERR(keypad->clk);
}
/* Init the Input device */
input_dev->name = pdev->name;
input_dev->id.bustype = BUS_HOST;
- input_dev->dev.parent = &pdev->dev;
+ input_dev->dev.parent = dev;
input_dev->open = imx_keypad_open;
input_dev->close = imx_keypad_close;
@@ -474,7 +475,7 @@ static int imx_keypad_probe(struct platform_device *pdev)
MAX_MATRIX_KEY_COLS,
keypad->keycodes, input_dev);
if (error) {
- dev_err(&pdev->dev, "failed to build keymap\n");
+ dev_err(dev, "failed to build keymap\n");
return error;
}
@@ -488,8 +489,8 @@ static int imx_keypad_probe(struct platform_device *pdev)
}
}
}
- dev_dbg(&pdev->dev, "enabled rows mask: %x\n", keypad->rows_en_mask);
- dev_dbg(&pdev->dev, "enabled cols mask: %x\n", keypad->cols_en_mask);
+ dev_dbg(dev, "enabled rows mask: %x\n", keypad->rows_en_mask);
+ dev_dbg(dev, "enabled cols mask: %x\n", keypad->cols_en_mask);
__set_bit(EV_REP, input_dev->evbit);
input_set_capability(input_dev, EV_MSC, MSC_SCAN);
@@ -502,22 +503,22 @@ static int imx_keypad_probe(struct platform_device *pdev)
imx_keypad_inhibit(keypad);
clk_disable_unprepare(keypad->clk);
- error = devm_request_irq(&pdev->dev, irq, imx_keypad_irq_handler, 0,
+ error = devm_request_irq(dev, irq, imx_keypad_irq_handler, 0,
pdev->name, keypad);
if (error) {
- dev_err(&pdev->dev, "failed to request IRQ\n");
+ dev_err(dev, "failed to request IRQ\n");
return error;
}
/* Register the input device */
error = input_register_device(input_dev);
if (error) {
- dev_err(&pdev->dev, "failed to register input device\n");
+ dev_err(dev, "failed to register input device\n");
return error;
}
platform_set_drvdata(pdev, keypad);
- device_init_wakeup(&pdev->dev, 1);
+ device_init_wakeup(dev, 1);
return 0;
}
--
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] input: touchscreen: imx6ul_tsc: Use 'dev' instead of dereferencing it repeatedly
From: Anson Huang @ 2019-09-24 9:02 UTC (permalink / raw)
To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam, swboyd,
mojha, linux-input, linux-arm-kernel, linux-kernel
Cc: Linux-imx
Add helper variable dev = &pdev->dev to simply the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/input/touchscreen/imx6ul_tsc.c | 37 +++++++++++++++++-----------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/imx6ul_tsc.c b/drivers/input/touchscreen/imx6ul_tsc.c
index 9ed2588..4555aa9 100644
--- a/drivers/input/touchscreen/imx6ul_tsc.c
+++ b/drivers/input/touchscreen/imx6ul_tsc.c
@@ -361,7 +361,8 @@ static void imx6ul_tsc_close(struct input_dev *input_dev)
static int imx6ul_tsc_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
struct imx6ul_tsc *tsc;
struct input_dev *input_dev;
int err;
@@ -369,11 +370,11 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
int adc_irq;
u32 average_samples;
- tsc = devm_kzalloc(&pdev->dev, sizeof(*tsc), GFP_KERNEL);
+ tsc = devm_kzalloc(dev, sizeof(*tsc), GFP_KERNEL);
if (!tsc)
return -ENOMEM;
- input_dev = devm_input_allocate_device(&pdev->dev);
+ input_dev = devm_input_allocate_device(dev);
if (!input_dev)
return -ENOMEM;
@@ -389,14 +390,14 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
input_set_drvdata(input_dev, tsc);
- tsc->dev = &pdev->dev;
+ tsc->dev = dev;
tsc->input = input_dev;
init_completion(&tsc->completion);
- tsc->xnur_gpio = devm_gpiod_get(&pdev->dev, "xnur", GPIOD_IN);
+ tsc->xnur_gpio = devm_gpiod_get(dev, "xnur", GPIOD_IN);
if (IS_ERR(tsc->xnur_gpio)) {
err = PTR_ERR(tsc->xnur_gpio);
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed to request GPIO tsc_X- (xnur): %d\n", err);
return err;
}
@@ -404,28 +405,28 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
tsc->tsc_regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(tsc->tsc_regs)) {
err = PTR_ERR(tsc->tsc_regs);
- dev_err(&pdev->dev, "failed to remap tsc memory: %d\n", err);
+ dev_err(dev, "failed to remap tsc memory: %d\n", err);
return err;
}
tsc->adc_regs = devm_platform_ioremap_resource(pdev, 1);
if (IS_ERR(tsc->adc_regs)) {
err = PTR_ERR(tsc->adc_regs);
- dev_err(&pdev->dev, "failed to remap adc memory: %d\n", err);
+ dev_err(dev, "failed to remap adc memory: %d\n", err);
return err;
}
- tsc->tsc_clk = devm_clk_get(&pdev->dev, "tsc");
+ tsc->tsc_clk = devm_clk_get(dev, "tsc");
if (IS_ERR(tsc->tsc_clk)) {
err = PTR_ERR(tsc->tsc_clk);
- dev_err(&pdev->dev, "failed getting tsc clock: %d\n", err);
+ dev_err(dev, "failed getting tsc clock: %d\n", err);
return err;
}
- tsc->adc_clk = devm_clk_get(&pdev->dev, "adc");
+ tsc->adc_clk = devm_clk_get(dev, "adc");
if (IS_ERR(tsc->adc_clk)) {
err = PTR_ERR(tsc->adc_clk);
- dev_err(&pdev->dev, "failed getting adc clock: %d\n", err);
+ dev_err(dev, "failed getting adc clock: %d\n", err);
return err;
}
@@ -439,18 +440,18 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
err = devm_request_threaded_irq(tsc->dev, tsc_irq,
NULL, tsc_irq_fn, IRQF_ONESHOT,
- dev_name(&pdev->dev), tsc);
+ dev_name(dev), tsc);
if (err) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed requesting tsc irq %d: %d\n",
tsc_irq, err);
return err;
}
err = devm_request_irq(tsc->dev, adc_irq, adc_irq_fn, 0,
- dev_name(&pdev->dev), tsc);
+ dev_name(dev), tsc);
if (err) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed requesting adc irq %d: %d\n",
adc_irq, err);
return err;
@@ -484,7 +485,7 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
tsc->average_select = ilog2(average_samples) - 2;
break;
default:
- dev_err(&pdev->dev,
+ dev_err(dev,
"touchscreen-average-samples (%u) must be 1, 4, 8, 16 or 32\n",
average_samples);
return -EINVAL;
@@ -492,7 +493,7 @@ static int imx6ul_tsc_probe(struct platform_device *pdev)
err = input_register_device(tsc->input);
if (err) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed to register input device: %d\n", err);
return err;
}
--
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] pwm: pwm-imx-tpm: Use 'dev' instead of dereferencing it repeatedly
From: Anson Huang @ 2019-09-24 9:01 UTC (permalink / raw)
To: thierry.reding, shawnguo, s.hauer, kernel, festevam, linux-pwm,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
Add helper variable dev = &pdev->dev to simply the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pwm/pwm-imx-tpm.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index e8385c1..4385801 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -337,11 +337,12 @@ static const struct pwm_ops imx_tpm_pwm_ops = {
static int pwm_imx_tpm_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct imx_tpm_pwm_chip *tpm;
int ret;
u32 val;
- tpm = devm_kzalloc(&pdev->dev, sizeof(*tpm), GFP_KERNEL);
+ tpm = devm_kzalloc(dev, sizeof(*tpm), GFP_KERNEL);
if (!tpm)
return -ENOMEM;
@@ -351,23 +352,23 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
if (IS_ERR(tpm->base))
return PTR_ERR(tpm->base);
- tpm->clk = devm_clk_get(&pdev->dev, NULL);
+ tpm->clk = devm_clk_get(dev, NULL);
if (IS_ERR(tpm->clk)) {
ret = PTR_ERR(tpm->clk);
if (ret != -EPROBE_DEFER)
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed to get PWM clock: %d\n", ret);
return ret;
}
ret = clk_prepare_enable(tpm->clk);
if (ret) {
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed to prepare or enable clock: %d\n", ret);
return ret;
}
- tpm->chip.dev = &pdev->dev;
+ tpm->chip.dev = dev;
tpm->chip.ops = &imx_tpm_pwm_ops;
tpm->chip.base = -1;
tpm->chip.of_xlate = of_pwm_xlate_with_flags;
@@ -381,7 +382,7 @@ static int pwm_imx_tpm_probe(struct platform_device *pdev)
ret = pwmchip_add(&tpm->chip);
if (ret) {
- dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
+ dev_err(dev, "failed to add PWM chip: %d\n", ret);
clk_disable_unprepare(tpm->clk);
}
--
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] pwm: pwm-imx27: Use 'dev' instead of dereferencing it repeatedly
From: Anson Huang @ 2019-09-24 8:59 UTC (permalink / raw)
To: thierry.reding, shawnguo, s.hauer, kernel, festevam, linux-pwm,
linux-arm-kernel, linux-kernel
Cc: Linux-imx
Add helper variable dev = &pdev->dev to simply the code.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pwm/pwm-imx27.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/pwm-imx27.c b/drivers/pwm/pwm-imx27.c
index 434a351..3afee29 100644
--- a/drivers/pwm/pwm-imx27.c
+++ b/drivers/pwm/pwm-imx27.c
@@ -290,27 +290,28 @@ MODULE_DEVICE_TABLE(of, pwm_imx27_dt_ids);
static int pwm_imx27_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct pwm_imx27_chip *imx;
- imx = devm_kzalloc(&pdev->dev, sizeof(*imx), GFP_KERNEL);
+ imx = devm_kzalloc(dev, sizeof(*imx), GFP_KERNEL);
if (imx == NULL)
return -ENOMEM;
platform_set_drvdata(pdev, imx);
- imx->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
+ imx->clk_ipg = devm_clk_get(dev, "ipg");
if (IS_ERR(imx->clk_ipg)) {
- dev_err(&pdev->dev, "getting ipg clock failed with %ld\n",
+ dev_err(dev, "getting ipg clock failed with %ld\n",
PTR_ERR(imx->clk_ipg));
return PTR_ERR(imx->clk_ipg);
}
- imx->clk_per = devm_clk_get(&pdev->dev, "per");
+ imx->clk_per = devm_clk_get(dev, "per");
if (IS_ERR(imx->clk_per)) {
int ret = PTR_ERR(imx->clk_per);
if (ret != -EPROBE_DEFER)
- dev_err(&pdev->dev,
+ dev_err(dev,
"failed to get peripheral clock: %d\n",
ret);
@@ -318,7 +319,7 @@ static int pwm_imx27_probe(struct platform_device *pdev)
}
imx->chip.ops = &pwm_imx27_ops;
- imx->chip.dev = &pdev->dev;
+ imx->chip.dev = dev;
imx->chip.base = -1;
imx->chip.npwm = 1;
--
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 v7 4/6] PM / devfreq: Introduce devfreq_get_freq_range
From: Leonard Crestez @ 2019-09-24 8:54 UTC (permalink / raw)
To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Alexandre Bailon,
Martin Kepplinger, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <e8d1d04a-1e8e-0765-179f-2146271787bb@samsung.com>
On 2019-09-24 8:09 AM, Chanwoo Choi wrote:
> Hi,
>
> On 19. 9. 24. 오전 6:10, Leonard Crestez wrote:
>> Moving handling of min/max freq to a single function and call it from
>> update_devfreq and for printing min/max freq values in sysfs.
>>
>> This changes the behavior of out-of-range min_freq/max_freq: clamping
>> is now done at evaluation time. This means that if an out-of-range
>> constraint is imposed by sysfs and it later becomes valid then it will
>> be enforced.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> drivers/devfreq/devfreq.c | 111 +++++++++++++++++++++-----------------
>> 1 file changed, 63 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 1cec816d3d00..7f152a582e78 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -24,10 +24,12 @@
>> #include <linux/printk.h>
>> #include <linux/hrtimer.h>
>> #include <linux/of.h>
>> #include "governor.h"
>>
>> +#define HZ_PER_KHZ 1000
>
> It is not used on this patch. If it is used on later patch,
> move it to other patch.
Good catch.
>> #define CREATE_TRACE_POINTS
>> #include <trace/events/devfreq.h>
>>
>> static struct class *devfreq_class;
>>
>> @@ -96,10 +98,50 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq)
>> dev_pm_opp_put(opp);
>>
>> return max_freq;
>> }
>>
>> +/**
>> + * devfreq_get_freq_range() - Get the current freq range
>> + * @devfreq: the devfreq instance
>> + * @min_freq: the min frequency
>> + * @max_freq: the max frequency
>> + *
>> + * This takes into consideration all constraints.
>> + */
>> +static void devfreq_get_freq_range(struct devfreq *devfreq,
>
> This function is used in the only devfreq core.
> I think that the internal function doesn't need to add 'devfreq' prefix.
> Also, when I developed the patches, I didn't use 'devfreq' prefix
> for internal function.
This does not appear to a clear convention, there are other internal
functions with a devfreq prefix. I usually keep prefixes always but it
does result in extra verbosity.
>> + unsigned long *min_freq,
>> + unsigned long *max_freq)
>> +{
>> + unsigned long *freq_table = devfreq->profile->freq_table;
>> +
>> + lockdep_assert_held(&devfreq->lock);
>> +
>> + /* Init min/max frequency from freq table */
>
> I think that the comments in the devfreq_get_freq_range(),
> it is not necessary. But if you think that it is necessary,
> please add more detailed description like as following:
>
> /*
> * The devfreq recommend that freq_table must be generated
> * in ascending order but, some devfreq driver used the descending order
> * for freq_table. In order to support all cases, check the order
> * of freq_table and then initialize the min/max frequency from freq_table.
> */
OK. Not strictly related to this series: wouldn't it make sense to just
fix the drivers? In linux kernel API compatibility hacks are usually
avoided.
>> + if (freq_table[0] < freq_table[devfreq->profile->max_state - 1]) {
>> + *min_freq = freq_table[0];
>> + *max_freq = freq_table[devfreq->profile->max_state - 1];
>> + } else {
>> + *min_freq = freq_table[devfreq->profile->max_state - 1];
>> + *max_freq = freq_table[0];
>> + }
>> +
>> + /* constraints from sysfs */
>
> ditto. Need to add more detailed comment by keeping the comment style of devfreq.
What comment style, capitalization and punctuation? This is similar to
the rest of the file.
>> + *min_freq = max(*min_freq, devfreq->min_freq);
>> + *max_freq = min(*max_freq, devfreq->max_freq);
>> +
>> + /* constraints from OPP interface */
>
> ditto.
>
>> + *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);
>> +
>> + /* max_freq takes precedence over min_freq */
>
> It is not necessary. We can know that min_freq have to be
> under max_freq without any comment.
>
>> + if (*min_freq > *max_freq)
>> + *min_freq = *max_freq;
This is a deliberate change introduced by this patch: instead of
ensuring min_freq <= max_freq at sysfs write time we make min_freq take
precedence at evaluation time.
With PM QoS there is no way to ensure that the value coming through
DEV_PM_QOS_MIN_FREQ is less than through DEV_PM_QOS_MAX_FREQ.
>> +}
>> +
>> /**
>> * devfreq_get_freq_level() - Lookup freq_table for the frequency
>> * @devfreq: the devfreq instance
>> * @freq: the target frequency
>> */
>> @@ -349,21 +391,13 @@ int update_devfreq(struct devfreq *devfreq)
>>
>> /* Reevaluate the proper frequency */
>> err = devfreq->governor->get_target_freq(devfreq, &freq);
>> if (err)
>> return err;
>> + devfreq_get_freq_range(devfreq, &min_freq, &max_freq);
>>
>> - /*
>> - * Adjust the frequency with user freq, QoS and available freq.
>> - *
>> - * List from the highest priority
>> - * max_freq
>> - * min_freq
>> - */
>> - max_freq = min(devfreq->scaling_max_freq, devfreq->max_freq);
>> - min_freq = max(devfreq->scaling_min_freq, devfreq->min_freq);
>> -
>> + /* max freq takes priority over min freq */
>
> Please remove it.
OK
>> if (freq < min_freq) {
>> freq = min_freq;
>> flags &= ~DEVFREQ_FLAG_LEAST_UPPER_BOUND; /* Use GLB */
>> }
>> if (freq > max_freq) {
>> @@ -1278,40 +1312,28 @@ 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);
>> -
>> - if (value) {
>> - if (value > df->max_freq) {
>> - ret = -EINVAL;
>> - goto unlock;
>> - }
>> - } else {
>> - unsigned long *freq_table = df->profile->freq_table;
>> -
>> - /* Get minimum frequency according to sorting order */
>> - if (freq_table[0] < freq_table[df->profile->max_state - 1])
>> - value = freq_table[0];
>> - else
>> - value = freq_table[df->profile->max_state - 1];
>> - }
>> -> df->min_freq = value;> update_devfreq(df);
>> - ret = count;
>> -unlock:
>> mutex_unlock(&df->lock);
>> - return ret;
>> +
>> + return count;
>> }
>>
>> static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> struct devfreq *df = to_devfreq(dev);
>> + unsigned long min_freq, max_freq;
>>
>> - return sprintf(buf, "%lu\n", max(df->scaling_min_freq, df->min_freq));
>> + mutex_lock(&df->lock);
>> + devfreq_get_freq_range(df, &min_freq, &max_freq);
>> + mutex_unlock(&df->lock);
>> +
>> + return sprintf(buf, "%lu\n", min_freq);
>> }
>>
>> static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> @@ -1323,40 +1345,33 @@ static ssize_t max_freq_store(struct device *dev, struct device_attribute *attr,
>> if (ret != 1)
>> return -EINVAL;
>>
>> mutex_lock(&df->lock);
>>
>> - if (value) {
>> - if (value < df->min_freq) {
>> - ret = -EINVAL;
>> - goto unlock;
>> - }
>> - } else {
>> - unsigned long *freq_table = df->profile->freq_table;
>> -
>> - /* Get maximum frequency according to sorting order */
>> - if (freq_table[0] < freq_table[df->profile->max_state - 1])
>> - value = freq_table[df->profile->max_state - 1];
>> - else
>> - value = freq_table[0];
>> - }
>> + /* Interpret zero as "don't care" */
>
> Please remove this comment.
This is documented in Documentation/ABI/testing/sysfs-class-devfreq but
not immediately obvious. Setting max to "zero" could have meant "min
possible" instead of "don't care".
>> + if (!value)
>> + value = ULONG_MAX;
>>
>> df->max_freq = value;
>> update_devfreq(df);
>> - ret = count;
>> -unlock:
>> mutex_unlock(&df->lock);
>> - return ret;
>> +
>> + return count;
>> }
>> static DEVICE_ATTR_RW(min_freq);
>>
>> static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> struct devfreq *df = to_devfreq(dev);
>> + unsigned long min_freq, max_freq;
>> +
>> + mutex_lock(&df->lock);
>> + devfreq_get_freq_range(df, &min_freq, &max_freq);
>> + mutex_unlock(&df->lock);
>>
>> - return sprintf(buf, "%lu\n", min(df->scaling_max_freq, df->max_freq));
>> + return sprintf(buf, "%lu\n", max_freq);
>> }
>> static DEVICE_ATTR_RW(max_freq);
>>
>> static ssize_t available_frequencies_show(struct device *d,
>> struct device_attribute *attr,
_______________________________________________
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 2/2] arm64/mm: Enable memory hot remove
From: Anshuman Khandual @ 2019-09-24 8:41 UTC (permalink / raw)
To: Matthew Wilcox
Cc: mark.rutland, mhocko, david, catalin.marinas, linux-mm, arunks,
cpandya, will, ira.weiny, steven.price, valentin.schneider,
suzuki.poulose, Robin.Murphy, broonie, cai, ard.biesheuvel,
dan.j.williams, linux-arm-kernel, osalvador, steve.capper, logang,
linux-kernel, akpm, mgorman
In-Reply-To: <20190923111745.GG15392@bombadil.infradead.org>
On 09/23/2019 04:47 PM, Matthew Wilcox wrote:
> On Mon, Sep 23, 2019 at 11:13:45AM +0530, Anshuman Khandual wrote:
>> +#ifdef CONFIG_MEMORY_HOTPLUG
>> +static void free_hotplug_page_range(struct page *page, size_t size)
>> +{
>> + WARN_ON(!page || PageReserved(page));
>
> WARN_ON(!page) isn't terribly useful. You're going to crash on the very
> next line when you call page_address() anyway. If this line were
>
> if (WARN_ON(!page || PageReserved(page)))
> return;
>
> it would make sense, or if it were just
>
> WARN_ON(PageReserved(page))
>
> it would also make sense.
I guess WARN_ON(PageReserved(page)) should be good enough to make sure
that page being freed here was originally allocated at runtime for a
previous memory hot add operation and did not some how come from the
memblock reserved area. That was the original objective for this check.
Will change it.
>
>> + free_pages((unsigned long)page_address(page), get_order(size));
>> +}
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 2/2] crypto: sun4i-ss: enable pm_runtime
From: Corentin Labbe @ 2019-09-24 8:08 UTC (permalink / raw)
To: davem, herbert, mripard, wens
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190924080832.18694-1-clabbe.montjoie@gmail.com>
This patch enables power management on the Security System.
sun4i-ss now depends on PM because it simplify code and prevent some ifdef.
But this is not a problem since arch maintainer want ARCH_SUNXI to
depend on PM in the future.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
drivers/crypto/Kconfig | 1 +
drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 10 +++
drivers/crypto/sunxi-ss/sun4i-ss-core.c | 82 ++++++++++++++++++-----
drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 12 ++++
drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 9 ++-
drivers/crypto/sunxi-ss/sun4i-ss.h | 2 +
6 files changed, 97 insertions(+), 19 deletions(-)
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 980b0844918b..ee53660b532c 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -662,6 +662,7 @@ config CRYPTO_DEV_IMGTEC_HASH
config CRYPTO_DEV_SUN4I_SS
tristate "Support for Allwinner Security System cryptographic accelerator"
depends on ARCH_SUNXI && !64BIT
+ depends on PM
select CRYPTO_MD5
select CRYPTO_SHA1
select CRYPTO_AES
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
index fa4b1b47822e..93acec22e42f 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-cipher.c
@@ -480,6 +480,7 @@ int sun4i_ss_cipher_init(struct crypto_tfm *tfm)
struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
struct sun4i_ss_alg_template *algt;
const char *name = crypto_tfm_alg_name(tfm);
+ int err;
memset(op, 0, sizeof(struct sun4i_tfm_ctx));
@@ -497,13 +498,22 @@ int sun4i_ss_cipher_init(struct crypto_tfm *tfm)
return PTR_ERR(op->fallback_tfm);
}
+ err = pm_runtime_get_sync(op->ss->dev);
+ if (err < 0)
+ goto error_pm;
+
return 0;
+error_pm:
+ crypto_free_sync_skcipher(op->fallback_tfm);
+ return err;
}
void sun4i_ss_cipher_exit(struct crypto_tfm *tfm)
{
struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
+
crypto_free_sync_skcipher(op->fallback_tfm);
+ pm_runtime_put(op->ss->dev);
}
/* check and set the AES key, prepare the mode to be used */
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 6c2db5d83b06..814cd12149a9 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -44,7 +44,8 @@ static struct sun4i_ss_alg_template ss_algs[] = {
.cra_blocksize = MD5_HMAC_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct sun4i_req_ctx),
.cra_module = THIS_MODULE,
- .cra_init = sun4i_hash_crainit
+ .cra_init = sun4i_hash_crainit,
+ .cra_exit = sun4i_hash_craexit,
}
}
}
@@ -70,7 +71,8 @@ static struct sun4i_ss_alg_template ss_algs[] = {
.cra_blocksize = SHA1_BLOCK_SIZE,
.cra_ctxsize = sizeof(struct sun4i_req_ctx),
.cra_module = THIS_MODULE,
- .cra_init = sun4i_hash_crainit
+ .cra_init = sun4i_hash_crainit,
+ .cra_exit = sun4i_hash_craexit,
}
}
}
@@ -223,17 +225,26 @@ static struct sun4i_ss_alg_template ss_algs[] = {
#endif
};
-static void sun4i_ss_disable(struct sun4i_ss_ctx *ss)
+/*
+ * Power management strategy: The device is suspended unless a TFM exists for
+ * one of the algorithms proposed by this driver.
+ */
+static int sun4i_ss_pm_suspend(struct device *dev)
{
+ struct sun4i_ss_ctx *ss = dev_get_drvdata(dev);
+
if (ss->reset)
reset_control_assert(ss->reset);
clk_disable_unprepare(ss->ssclk);
clk_disable_unprepare(ss->busclk);
+ return 0;
}
-static int sun4i_ss_enable(struct sun4i_ss_ctx *ss)
+static int sun4i_ss_pm_resume(struct device *dev)
{
+ struct sun4i_ss_ctx *ss = dev_get_drvdata(dev);
+
int err;
err = clk_prepare_enable(ss->busclk);
@@ -258,10 +269,38 @@ static int sun4i_ss_enable(struct sun4i_ss_ctx *ss)
return err;
err_enable:
- sun4i_ss_disable(ss);
+ sun4i_ss_pm_suspend(dev);
+ return err;
+}
+
+const struct dev_pm_ops sun4i_ss_pm_ops = {
+ SET_RUNTIME_PM_OPS(sun4i_ss_pm_suspend, sun4i_ss_pm_resume, NULL)
+};
+
+/*
+ * When power management is enabled, this function enables the PM and set the
+ * device as suspended
+ * When power management is disabled, this function just enables the device
+ */
+static int sun4i_ss_pm_init(struct sun4i_ss_ctx *ss)
+{
+ int err;
+
+ pm_runtime_use_autosuspend(ss->dev);
+ pm_runtime_set_autosuspend_delay(ss->dev, 2000);
+
+ err = pm_runtime_set_suspended(ss->dev);
+ if (err)
+ return err;
+ pm_runtime_enable(ss->dev);
return err;
}
+static void sun4i_ss_pm_exit(struct sun4i_ss_ctx *ss)
+{
+ pm_runtime_disable(ss->dev);
+}
+
static int sun4i_ss_probe(struct platform_device *pdev)
{
u32 v;
@@ -308,10 +347,6 @@ static int sun4i_ss_probe(struct platform_device *pdev)
ss->reset = NULL;
}
- err = sun4i_ss_enable(ss);
- if (err)
- goto error_enable;
-
/*
* Check that clock have the correct rates given in the datasheet
* Try to set the clock to the maximum allowed
@@ -319,7 +354,7 @@ static int sun4i_ss_probe(struct platform_device *pdev)
err = clk_set_rate(ss->ssclk, cr_mod);
if (err) {
dev_err(&pdev->dev, "Cannot set clock rate to ssclk\n");
- goto error_enable;
+ return err;
}
/*
@@ -347,12 +382,26 @@ static int sun4i_ss_probe(struct platform_device *pdev)
dev_warn(&pdev->dev, "Clock ss is at %lu (%lu MHz) (must be <= %lu)\n",
cr, cr / 1000000, cr_mod);
+ ss->dev = &pdev->dev;
+ platform_set_drvdata(pdev, ss);
+
+ spin_lock_init(&ss->slock);
+
+ err = sun4i_ss_pm_init(ss);
+ if (err)
+ return err;
+
/*
* Datasheet named it "Die Bonding ID"
* I expect to be a sort of Security System Revision number.
* Since the A80 seems to have an other version of SS
* this info could be useful
*/
+
+ err = pm_runtime_get_sync(ss->dev);
+ if (err < 0)
+ goto error_pm;
+
writel(SS_ENABLED, ss->base + SS_CTL);
v = readl(ss->base + SS_CTL);
v >>= 16;
@@ -360,9 +409,7 @@ static int sun4i_ss_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "Die ID %d\n", v);
writel(0, ss->base + SS_CTL);
- ss->dev = &pdev->dev;
-
- spin_lock_init(&ss->slock);
+ pm_runtime_put_sync(ss->dev);
for (i = 0; i < ARRAY_SIZE(ss_algs); i++) {
ss_algs[i].ss = ss;
@@ -392,7 +439,6 @@ static int sun4i_ss_probe(struct platform_device *pdev)
break;
}
}
- platform_set_drvdata(pdev, ss);
return 0;
error_alg:
i--;
@@ -409,8 +455,8 @@ static int sun4i_ss_probe(struct platform_device *pdev)
break;
}
}
-error_enable:
- sun4i_ss_disable(ss);
+error_pm:
+ sun4i_ss_pm_exit(ss);
return err;
}
@@ -433,8 +479,7 @@ static int sun4i_ss_remove(struct platform_device *pdev)
}
}
- writel(0, ss->base + SS_CTL);
- sun4i_ss_disable(ss);
+ sun4i_ss_pm_exit(ss);
return 0;
}
@@ -449,6 +494,7 @@ static struct platform_driver sun4i_ss_driver = {
.remove = sun4i_ss_remove,
.driver = {
.name = "sun4i-ss",
+ .pm = &sun4i_ss_pm_ops,
.of_match_table = a20ss_crypto_of_match_table,
},
};
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
index fcffba5ef927..9930c9ce8971 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-hash.c
@@ -19,17 +19,29 @@ int sun4i_hash_crainit(struct crypto_tfm *tfm)
struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
struct ahash_alg *alg = __crypto_ahash_alg(tfm->__crt_alg);
struct sun4i_ss_alg_template *algt;
+ int err;
memset(op, 0, sizeof(struct sun4i_tfm_ctx));
algt = container_of(alg, struct sun4i_ss_alg_template, alg.hash);
op->ss = algt->ss;
+ err = pm_runtime_get_sync(op->ss->dev);
+ if (err < 0)
+ return err;
+
crypto_ahash_set_reqsize(__crypto_ahash_cast(tfm),
sizeof(struct sun4i_req_ctx));
return 0;
}
+void sun4i_hash_craexit(struct crypto_tfm *tfm)
+{
+ struct sun4i_tfm_ctx *op = crypto_tfm_ctx(tfm);
+
+ pm_runtime_put(op->ss->dev);
+}
+
/* sun4i_hash_init: initialize request context */
int sun4i_hash_init(struct ahash_request *areq)
{
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
index 63d636424161..729aafdbea84 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
@@ -17,7 +17,7 @@ int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
{
struct sun4i_ss_alg_template *algt;
struct rng_alg *alg = crypto_rng_alg(tfm);
- int i;
+ int i, err;
u32 v;
u32 *data = (u32 *)dst;
const u32 mode = SS_OP_PRNG | SS_PRNG_CONTINUE | SS_ENABLED;
@@ -28,6 +28,10 @@ int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
ss = algt->ss;
+ err = pm_runtime_get_sync(ss->dev);
+ if (err < 0)
+ return err;
+
spin_lock_bh(&ss->slock);
writel(mode, ss->base + SS_CTL);
@@ -52,5 +56,8 @@ int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
writel(0, ss->base + SS_CTL);
spin_unlock_bh(&ss->slock);
+
+ pm_runtime_put(ss->dev);
+
return 0;
}
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss.h b/drivers/crypto/sunxi-ss/sun4i-ss.h
index 35a27a7145f8..60425ac75d90 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss.h
+++ b/drivers/crypto/sunxi-ss/sun4i-ss.h
@@ -22,6 +22,7 @@
#include <linux/scatterlist.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
+#include <linux/pm_runtime.h>
#include <crypto/md5.h>
#include <crypto/skcipher.h>
#include <crypto/sha.h>
@@ -177,6 +178,7 @@ struct sun4i_req_ctx {
};
int sun4i_hash_crainit(struct crypto_tfm *tfm);
+void sun4i_hash_craexit(struct crypto_tfm *tfm);
int sun4i_hash_init(struct ahash_request *areq);
int sun4i_hash_update(struct ahash_request *areq);
int sun4i_hash_final(struct ahash_request *areq);
--
2.21.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 v3 1/2] crypto: sun4i-ss: simplify enable/disable of the device
From: Corentin Labbe @ 2019-09-24 8:08 UTC (permalink / raw)
To: davem, herbert, mripard, wens
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
In-Reply-To: <20190924080832.18694-1-clabbe.montjoie@gmail.com>
This patch regroups resource enabling/disabling in dedicated function.
This simplify error handling and will permit to support power
management.
Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
Acked-by: Maxime Ripard <mripard@kernel.org>
---
drivers/crypto/sunxi-ss/sun4i-ss-core.c | 77 +++++++++++++++----------
1 file changed, 46 insertions(+), 31 deletions(-)
diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-core.c b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
index 9aa6fe081a27..6c2db5d83b06 100644
--- a/drivers/crypto/sunxi-ss/sun4i-ss-core.c
+++ b/drivers/crypto/sunxi-ss/sun4i-ss-core.c
@@ -223,6 +223,45 @@ static struct sun4i_ss_alg_template ss_algs[] = {
#endif
};
+static void sun4i_ss_disable(struct sun4i_ss_ctx *ss)
+{
+ if (ss->reset)
+ reset_control_assert(ss->reset);
+
+ clk_disable_unprepare(ss->ssclk);
+ clk_disable_unprepare(ss->busclk);
+}
+
+static int sun4i_ss_enable(struct sun4i_ss_ctx *ss)
+{
+ int err;
+
+ err = clk_prepare_enable(ss->busclk);
+ if (err) {
+ dev_err(ss->dev, "Cannot prepare_enable busclk\n");
+ goto err_enable;
+ }
+
+ err = clk_prepare_enable(ss->ssclk);
+ if (err) {
+ dev_err(ss->dev, "Cannot prepare_enable ssclk\n");
+ goto err_enable;
+ }
+
+ if (ss->reset) {
+ err = reset_control_deassert(ss->reset);
+ if (err) {
+ dev_err(ss->dev, "Cannot deassert reset control\n");
+ goto err_enable;
+ }
+ }
+
+ return err;
+err_enable:
+ sun4i_ss_disable(ss);
+ return err;
+}
+
static int sun4i_ss_probe(struct platform_device *pdev)
{
u32 v;
@@ -269,17 +308,9 @@ static int sun4i_ss_probe(struct platform_device *pdev)
ss->reset = NULL;
}
- /* Enable both clocks */
- err = clk_prepare_enable(ss->busclk);
- if (err) {
- dev_err(&pdev->dev, "Cannot prepare_enable busclk\n");
- return err;
- }
- err = clk_prepare_enable(ss->ssclk);
- if (err) {
- dev_err(&pdev->dev, "Cannot prepare_enable ssclk\n");
- goto error_ssclk;
- }
+ err = sun4i_ss_enable(ss);
+ if (err)
+ goto error_enable;
/*
* Check that clock have the correct rates given in the datasheet
@@ -288,16 +319,7 @@ static int sun4i_ss_probe(struct platform_device *pdev)
err = clk_set_rate(ss->ssclk, cr_mod);
if (err) {
dev_err(&pdev->dev, "Cannot set clock rate to ssclk\n");
- goto error_clk;
- }
-
- /* Deassert reset if we have a reset control */
- if (ss->reset) {
- err = reset_control_deassert(ss->reset);
- if (err) {
- dev_err(&pdev->dev, "Cannot deassert reset control\n");
- goto error_clk;
- }
+ goto error_enable;
}
/*
@@ -387,12 +409,8 @@ static int sun4i_ss_probe(struct platform_device *pdev)
break;
}
}
- if (ss->reset)
- reset_control_assert(ss->reset);
-error_clk:
- clk_disable_unprepare(ss->ssclk);
-error_ssclk:
- clk_disable_unprepare(ss->busclk);
+error_enable:
+ sun4i_ss_disable(ss);
return err;
}
@@ -416,10 +434,7 @@ static int sun4i_ss_remove(struct platform_device *pdev)
}
writel(0, ss->base + SS_CTL);
- if (ss->reset)
- reset_control_assert(ss->reset);
- clk_disable_unprepare(ss->busclk);
- clk_disable_unprepare(ss->ssclk);
+ sun4i_ss_disable(ss);
return 0;
}
--
2.21.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 v3 0/2] crypto: sun4i-ss: Enable power management
From: Corentin Labbe @ 2019-09-24 8:08 UTC (permalink / raw)
To: davem, herbert, mripard, wens
Cc: linux-sunxi, Corentin Labbe, linux-crypto, linux-arm-kernel,
linux-kernel
Hello
This serie enables power management in the sun4i-ss driver.
Regards
Changes since v2 ( https://lore.kernel.org/linux-arm-kernel/20190919051035.4111-2-clabbe.montjoie@gmail.com/T/ ):
- depends on PM
- fusioned suspend/resume functions with sun4i_ssenable/disable
- fixed style problem
Changes since v1:
- Fixed style in patch #1
- Check more return code of PM functions
- Add PM support in hash/prng
- reworked the probe order of PM functions and the PM strategy
Corentin Labbe (2):
crypto: sun4i-ss: simplify enable/disable of the device
crypto: sun4i-ss: enable pm_runtime
drivers/crypto/Kconfig | 1 +
drivers/crypto/sunxi-ss/sun4i-ss-cipher.c | 10 ++
drivers/crypto/sunxi-ss/sun4i-ss-core.c | 139 ++++++++++++++++------
drivers/crypto/sunxi-ss/sun4i-ss-hash.c | 12 ++
drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 9 +-
drivers/crypto/sunxi-ss/sun4i-ss.h | 2 +
6 files changed, 133 insertions(+), 40 deletions(-)
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [EXT] Re: [PATCH] arm64: dts: ls1028a: fix a compatible issue
From: Andy Tang @ 2019-09-24 8:00 UTC (permalink / raw)
To: Shawn Guo
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org, Leo Li
In-Reply-To: <20190914024231.GB3425@dragon>
Hi Shawn,
> -----Original Message-----
> From: Shawn Guo <shawnguo@kernel.org>
> Sent: 2019年9月14日 10:43
> To: Andy Tang <andy.tang@nxp.com>
> Cc: Leo Li <leoyang.li@nxp.com>; robh+dt@kernel.org;
> mark.rutland@arm.com; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org
> Subject: [EXT] Re: [PATCH] arm64: dts: ls1028a: fix a compatible issue
>
> Caution: EXT Email
>
> On Thu, Aug 29, 2019 at 03:34:39PM +0800, Yuantian Tang wrote:
> > The I2C multiplexer used on ls1028aqds is PCA9547, not PCA9847.
> > So correct it.
>
> Can elaborate the fix a little bit, saying what's the issue exactly with using
> that incorrect compatible?
Ok.
>
> >
> > Signed-off-by: Yuantian Tang <andy.tang@nxp.com>
>
> Do we need a fix tag?
What does "fix tag" mean?
BR,
Andy
>
> Shawn
>
> > ---
> > arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > index 5e14e5a19744..f5da9e8b0d9d 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-qds.dts
> > @@ -107,7 +107,7 @@
> > status = "okay";
> >
> > i2c-mux@77 {
> > - compatible = "nxp,pca9847";
> > + compatible = "nxp,pca9547";
> > reg = <0x77>;
> > #address-cells = <1>;
> > #size-cells = <0>;
> > --
> > 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 v7 2/6] PM / devfreq: Move more initialization before registration
From: Leonard Crestez @ 2019-09-24 8:00 UTC (permalink / raw)
To: Chanwoo Choi, MyungJoo Ham, Kyungmin Park, Matthias Kaehlcke
Cc: Artur Świgoń, Abel Vesa, Saravana Kannan,
linux-pm@vger.kernel.org, Viresh Kumar, dl-linux-imx,
Krzysztof Kozlowski, Lukasz Luba, Alexandre Bailon,
Martin Kepplinger, Georgi Djakov,
linux-arm-kernel@lists.infradead.org, Jacky Bai
In-Reply-To: <bfa991bf-7155-6294-0595-fc37e3393fdd@samsung.com>
On 2019-09-24 5:52 AM, Chanwoo Choi wrote:
> Hi,
>
> I'm not sure that it is necessary. Because, mainline kernel
> provides the resource-managed feature with 'devm_*'
> in order to reduce the memory leak by user.
>
> It's like going back to a time when 'devm_*' feature wasn't there.
> I think just it depends on the coding style of author.
>
> I leave the final decision of this patch to Myungjoo.
Initializing before registration is a dependency of the next patch which
relaxes locking on init path.
Dropping devm is indeed odd but manual memory management in a subsystem
core seems reasonable. I think devm could be kept around by spliting
device_register into separate device_initialize and device_add steps.
> On 19. 9. 24. 오전 6:10, Leonard Crestez wrote:
>> In general it is a better to initialize an object before making it
>> accessible externally (through device_register).
>>
>> This makes it possible to avoid relying on locking a partially
>> initialized object.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>> ---
>> drivers/devfreq/devfreq.c | 43 +++++++++++++++++++++++----------------
>> 1 file changed, 25 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index f5bea7350f72..8bbcd4efa09f 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -588,10 +588,12 @@ static void devfreq_dev_release(struct device *dev)
>> mutex_unlock(&devfreq_list_lock);
>>
>> if (devfreq->profile->exit)
>> devfreq->profile->exit(devfreq->dev.parent);
>>
>> + kfree(devfreq->time_in_state);
>> + kfree(devfreq->trans_table);
>> mutex_destroy(&devfreq->lock);
>> kfree(devfreq);
>> }
>>
>> /**
>> @@ -671,44 +673,43 @@ struct devfreq *devfreq_add_device(struct device *dev,
>> devfreq->max_freq = devfreq->scaling_max_freq;
>>
>> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
>> atomic_set(&devfreq->suspend_count, 0);
>>
>> - dev_set_name(&devfreq->dev, "devfreq%d",
>> - atomic_inc_return(&devfreq_no));
>> - err = device_register(&devfreq->dev);
>> - if (err) {
>> - mutex_unlock(&devfreq->lock);
>> - put_device(&devfreq->dev);
>> - goto err_out;
>> - }
>> -
>> - devfreq->trans_table = devm_kzalloc(&devfreq->dev,
>> + devfreq->trans_table = kzalloc(
>> array3_size(sizeof(unsigned int),
>> devfreq->profile->max_state,
>> devfreq->profile->max_state),
>> GFP_KERNEL);
>> if (!devfreq->trans_table) {
>> mutex_unlock(&devfreq->lock);
>> err = -ENOMEM;
>> - goto err_devfreq;
>> + goto err_dev;
>> }
>>
>> - devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
>> - devfreq->profile->max_state,
>> - sizeof(unsigned long),
>> - GFP_KERNEL);
>> + devfreq->time_in_state = kcalloc(devfreq->profile->max_state,
>> + sizeof(unsigned long),
>> + GFP_KERNEL);
>> if (!devfreq->time_in_state) {
>> mutex_unlock(&devfreq->lock);
>> err = -ENOMEM;
>> - goto err_devfreq;
>> + goto err_dev;
>> }
>>
>> devfreq->last_stat_updated = jiffies;
>>
>> srcu_init_notifier_head(&devfreq->transition_notifier_list);
>>
>> + dev_set_name(&devfreq->dev, "devfreq%d",
>> + atomic_inc_return(&devfreq_no));
>> + err = device_register(&devfreq->dev);
>> + if (err) {
>> + mutex_unlock(&devfreq->lock);
>> + put_device(&devfreq->dev);
>> + goto err_out;
>> + }
>> +
>> mutex_unlock(&devfreq->lock);
>>
>> mutex_lock(&devfreq_list_lock);
>>
>> governor = try_then_request_governor(devfreq->governor_name);
>> @@ -734,14 +735,20 @@ struct devfreq *devfreq_add_device(struct device *dev,
>>
>> return devfreq;
>>
>> err_init:
>> mutex_unlock(&devfreq_list_lock);
>> -err_devfreq:
>> devfreq_remove_device(devfreq);
>> - devfreq = NULL;
>> + return ERR_PTR(err);
>> +
>> err_dev:
>> + /*
>> + * Cleanup path for errors that happen before registration.
>> + * Otherwise we rely on devfreq_dev_release.
>> + */
>> + kfree(devfreq->time_in_state);
>> + kfree(devfreq->trans_table);
>> kfree(devfreq);
>> err_out:
>> return ERR_PTR(err);
>> }
>> EXPORT_SYMBOL(devfreq_add_device);
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3] PM / devfreq: Lock devfreq in trans_stat_show
From: Leonard Crestez @ 2019-09-24 7:52 UTC (permalink / raw)
To: MyungJoo Ham, Kyungmin Park, Chanwoo Choi, Matthias Kaehlcke
Cc: Artur Świgoń, linux-pm, Krzysztof Kozlowski,
Lukasz Luba, linux-imx, linux-arm-kernel
There is no locking in this sysfs show function so stats printing can
race with a devfreq_update_status called as part of freq switching or
with initialization.
Also add an assert in devfreq_update_status to make it clear that lock
must be held by caller.
Fixes: 39688ce6facd ("PM / devfreq: account suspend/resume for stats")
Cc: stable@vger.kernel.org
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
---
Changes since v2:
* Move assert higher
* Drop useless comment
* Add Fixes: tag and Cc: stable
Link to v2: https://patchwork.kernel.org/patch/11157315/
Changes since v1:
* Split from series: low-priority bugfix not strictly required for PM QoS
* Only keep lock during update, release before sprintf
Link to v1: https://patchwork.kernel.org/patch/11149493/
drivers/devfreq/devfreq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 072c0731659d..a2a045e117f0 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -158,10 +158,11 @@ static int set_freq_table(struct devfreq *devfreq)
int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
{
int lev, prev_lev, ret = 0;
unsigned long cur_time;
+ lockdep_assert_held(&devfreq->lock);
cur_time = jiffies;
/* Immediately exit if previous_freq is not initialized yet. */
if (!devfreq->previous_freq)
goto out;
@@ -1415,16 +1416,21 @@ static ssize_t trans_stat_show(struct device *dev,
struct devfreq *devfreq = to_devfreq(dev);
ssize_t len;
int i, j;
unsigned int max_state = devfreq->profile->max_state;
- if (!devfreq->stop_polling &&
- devfreq_update_status(devfreq, devfreq->previous_freq))
- return 0;
if (max_state == 0)
return sprintf(buf, "Not Supported.\n");
+ mutex_lock(&devfreq->lock);
+ if (!devfreq->stop_polling &&
+ devfreq_update_status(devfreq, devfreq->previous_freq)) {
+ mutex_unlock(&devfreq->lock);
+ return 0;
+ }
+ mutex_unlock(&devfreq->lock);
+
len = sprintf(buf, " From : To\n");
len += sprintf(buf + len, " :");
for (i = 0; i < max_state; i++)
len += sprintf(buf + len, "%10lu",
devfreq->profile->freq_table[i]);
--
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
* [PATCH v2 22/22] mtd: spi-nor: Rework the disabling of block write protection
From: Tudor.Ambarus @ 2019-09-24 7:47 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
spi_nor_unlock() unlocks blocks of memory or the entire flash memory
array, if requested. clear_sr_bp() unlocks the entire flash memory
array at boot time. This calls for some unification, clear_sr_bp() is
just an optimization for the case when the unlock request covers the
entire flash size.
Get rid of clear_sr_bp() and introduce spi_nor_unlock_all(), which is
just a call to spi_nor_unlock() for the entire flash memory array.
This fixes a bug that was present in spi_nor_spansion_clear_sr_bp().
When the QE bit was zero, we used the Write Status (01h) command with
one data byte, which might cleared the Status Register 2. We now always
use the Write Status (01h) command with two data bytes when
SNOR_F_HAS_16BIT_SR is set, to avoid clearing the Status Register 2.
The SNOR_F_NO_READ_CR case is treated as well. When the flash doesn't
support the CR Read command, we make an assumption about the value of
the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can
be sure the QE bit has value one, because of the previous call to
spi_nor_quad_enable().
Get rid of the MFR handling and implement specific manufacturer
default_init() fixup hooks.
Note that this changes a bit the logic for the SNOR_MFR_ATMEL,
SNOR_MFR_INTEL and SNOR_MFR_SST cases. Before this patch, the Atmel,
Intel and SST chips did not set the locking ops, but unlocked the entire
flash at boot time, while now they are setting the locking ops to
stm_locking_ops. This should work, since the the disable of the block
protection at the boot time used the same Status Register bits to unlock
the flash, as in the stm_locking_ops case.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 146 +++++++++++++++---------------------------
include/linux/mtd/spi-nor.h | 3 -
2 files changed, 51 insertions(+), 98 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a53e2cdc564c..7b61edf284a1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2085,79 +2085,6 @@ static int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
return 0;
}
-/**
- * spi_nor_clear_sr_bp() - clear the Status Register Block Protection bits.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_clear_sr_bp(struct spi_nor *nor)
-{
- int ret;
- u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- nor->bouncebuf[0] &= mask;
-
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
-
- return ret;
-}
-
-/**
- * spi_nor_spansion_clear_sr_bp() - clear the Status Register Block Protection
- * bits on spansion flashes.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits. The function is tightly
- * coupled with the spi_nor_sr2_bit1_quad_enable() function. Both assume that
- * the Write Register with 16 bits, together with the Read Configuration
- * Register (35h) instructions are supported.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
-{
- int ret;
- u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
- u8 *sr_cr = nor->bouncebuf;
-
- /* Check current Quad Enable bit value. */
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- return ret;
-
- /*
- * When the configuration register Quad Enable bit is one, only the
- * Write Status (01h) command with two data bytes may be used.
- */
- if (sr_cr[1] & SR2_QUAD_EN_BIT1) {
- ret = spi_nor_read_sr(nor, &sr_cr[0]);
- if (ret)
- return ret;
-
- sr_cr[0] &= ~mask;
-
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- dev_err(nor->dev, "16-bit write register failed\n");
- return ret;
- }
-
- /*
- * If the Quad Enable bit is zero, use the Write Status (01h) command
- * with one data byte.
- */
- return spi_nor_clear_sr_bp(nor);
-}
-
/* Used when the "_ext_id" is two bytes at most */
#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
.id = { \
@@ -4542,6 +4469,16 @@ static int spi_nor_setup(struct spi_nor *nor,
return nor->flash.setup(nor, hwcaps);
}
+static void atmel_set_default_init(struct spi_nor *nor)
+{
+ nor->flags |= SNOR_F_HAS_LOCK;
+}
+
+static void intel_set_default_init(struct spi_nor *nor)
+{
+ nor->flags |= SNOR_F_HAS_LOCK;
+}
+
static void macronix_set_default_init(struct spi_nor *nor)
{
nor->flash.quad_enable = spi_nor_sr1_bit6_quad_enable;
@@ -4555,6 +4492,11 @@ static void st_micron_set_default_init(struct spi_nor *nor)
nor->flash.set_4byte = st_micron_set_4byte;
}
+static void sst_set_default_init(struct spi_nor *nor)
+{
+ nor->flags |= SNOR_F_HAS_LOCK;
+}
+
static void winbond_set_default_init(struct spi_nor *nor)
{
nor->flash.set_4byte = winbond_set_4byte;
@@ -4569,6 +4511,14 @@ static void spi_nor_manufacturer_init_flash_params(struct spi_nor *nor)
{
/* Init flash parameters based on MFR */
switch (JEDEC_MFR(nor->info)) {
+ case SNOR_MFR_ATMEL:
+ atmel_set_default_init(nor);
+ break;
+
+ case SNOR_MFR_INTEL:
+ intel_set_default_init(nor);
+ break;
+
case SNOR_MFR_MACRONIX:
macronix_set_default_init(nor);
break;
@@ -4578,6 +4528,10 @@ static void spi_nor_manufacturer_init_flash_params(struct spi_nor *nor)
st_micron_set_default_init(nor);
break;
+ case SNOR_MFR_SST:
+ sst_set_default_init(nor);
+ break;
+
case SNOR_MFR_WINBOND:
winbond_set_default_init(nor);
break;
@@ -4839,21 +4793,26 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
return nor->flash.quad_enable(nor);
}
-static int spi_nor_init(struct spi_nor *nor)
+/**
+ * spi_nor_unlock_all() - Unlocks the entire flash memory array.
+ * @nor: pointer to a 'struct spi_nor'
+ *
+ * Some SPI NOR flashes are write protected by default after a power-on reset
+ * cycle, in order to avoid inadvertent writes during power-up. Backward
+ * compatibility imposes to unlock the entire flash memory array at power-up
+ * by default.
+ */
+static int spi_nor_unlock_all(struct spi_nor *nor)
{
- int err;
+ if (nor->flags & SNOR_F_HAS_LOCK)
+ return spi_nor_unlock(&nor->mtd, 0, nor->flash.size);
- if (nor->clear_sr_bp) {
- if (nor->flash.quad_enable == spi_nor_sr2_bit1_quad_enable)
- nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
+ return 0;
+}
- err = nor->clear_sr_bp(nor);
- if (err) {
- dev_err(nor->dev,
- "fail to clear block protection bits\n");
- return err;
- }
- }
+static int spi_nor_init(struct spi_nor *nor)
+{
+ int err;
err = spi_nor_quad_enable(nor);
if (err) {
@@ -4861,6 +4820,13 @@ static int spi_nor_init(struct spi_nor *nor)
return err;
}
+ err = spi_nor_unlock_all(nor);
+ if (err) {
+ dev_err(nor->dev,
+ "Failed to unlock the entire flash memory array\n");
+ return err;
+ }
+
if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
/*
* If the RESET# pin isn't hooked up properly, or the system
@@ -5043,16 +5009,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (info->flags & SPI_NOR_HAS_LOCK)
nor->flags |= SNOR_F_HAS_LOCK;
- /*
- * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
- * with the software protection bits set.
- */
- if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL ||
- JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
- JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
- nor->info->flags & SPI_NOR_HAS_LOCK)
- nor->clear_sr_bp = spi_nor_clear_sr_bp;
-
/* Init flash parameters based on flash_info struct and SFDP */
spi_nor_init_flash_params(nor);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 5590a36eb43e..ca650f895903 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -578,8 +578,6 @@ struct flash_info;
* @write_proto: the SPI protocol for write operations
* @reg_proto the SPI protocol for read_reg/write_reg/erase operations
* @controller_ops: SPI NOR controller driver specific operations.
- * @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
- * the SPI NOR Status Register.
* @flash: SPI-NOR flash parameters and settings. The structure
* includes default flash parameters and settings that can
* be overwritten by the spi_nor_fixups hooks, or
@@ -608,7 +606,6 @@ struct spi_nor {
const struct spi_nor_controller_ops *controller_ops;
- int (*clear_sr_bp)(struct spi_nor *nor);
struct spi_nor_flash_parameter flash;
void *priv;
--
2.9.5
_______________________________________________
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 v2 20/22] mtd: spi-nor: Rework spansion(_no)_read_cr_quad_enable()
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Merge:
spansion_no_read_cr_quad_enable()
spansion_read_cr_quad_enable()
in spi_nor_sr2_bit1_quad_enable().
Introduce spi_nor_write_16bit_cr_and_check(). The Configuration Register
contains bits that can be updated in future: FREEZE, CMP. Provide a
generic method that allows updating all bits of the Configuration
Register.
Do the Read Back test even for the old spansion_no_read_cr_quad_enable()
case.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 136 +++++++++++++++++++++---------------------
include/linux/mtd/spi-nor.h | 4 +-
2 files changed, 70 insertions(+), 70 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 112f93cec7ba..8fd1c04f75d9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -903,7 +903,7 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
* Write Status (01h) command is available just for the cases
* in which the QE bit is described in SR2 at BIT(1).
*/
- sr_cr[1] = CR_QUAD_EN_SPAN;
+ sr_cr[1] = SR2_QUAD_EN_BIT1;
} else {
sr_cr[1] = 0;
}
@@ -941,6 +941,59 @@ static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
}
/**
+ * spi_nor_write_16bit_cr_and_check() - Write the Status Register 1 and the
+ * Configuration Register in one shot. Ensure that the byte written in the
+ * Configuration Register match the received value, and that the 16-bit Write
+ * did not affect what was already in the Status Register 1.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @cr: byte value to be written to the Configuration Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_cr_and_check(struct spi_nor *nor, u8 cr)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+ u8 sr_written;
+
+ /* Keep the current value of the Status Register 1. */
+ ret = spi_nor_read_sr(nor, &sr_cr[0]);
+ if (ret)
+ return ret;
+
+ sr_cr[1] = cr;
+
+ ret = spi_nor_write_sr(nor, sr_cr, 2);
+ if (ret)
+ return ret;
+
+ sr_written = sr_cr[0];
+
+ ret = spi_nor_read_sr(nor, &sr_cr[0]);
+ if (ret)
+ return ret;
+
+ if (sr_written != sr_cr[0]) {
+ dev_err(nor->dev, "SR1: Read back test failed\n");
+ return -EIO;
+ }
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (cr != sr_cr[1]) {
+ dev_err(nor->dev, "CR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
* spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
* the byte written match the received value without affecting other bits in the
* Status Register 1 and 2.
@@ -1958,81 +2011,30 @@ static int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
}
/**
- * spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
+ * spi_nor_sr2_bit1_quad_enable() - set the Quad Enable BIT(1) in the Status
+ * Register 2.
* @nor: pointer to a 'struct spi_nor'
*
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories not supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
+ * Bit 1 of the Status Register 2 is the QE bit for Spansion like QSPI memories.
*
* Return: 0 on success, -errno otherwise.
*/
-static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
{
- u8 *sr_cr = nor->bouncebuf;
int ret;
- /* Keep the current value of the Status Register. */
- ret = spi_nor_read_sr(nor, &sr_cr[0]);
- if (ret)
- return ret;
-
- sr_cr[1] = CR_QUAD_EN_SPAN;
-
- return spi_nor_write_sr(nor, sr_cr, 2);
-}
-
-/**
- * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_read_cr_quad_enable(struct spi_nor *nor)
-{
- u8 *sr_cr = nor->bouncebuf;
- int ret;
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return spi_nor_write_16bit_cr_and_check(nor, SR2_QUAD_EN_BIT1);
/* Check current Quad Enable bit value. */
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ ret = spi_nor_read_cr(nor, &nor->bouncebuf[0]);
if (ret)
return ret;
- if (sr_cr[1] & CR_QUAD_EN_SPAN)
+ if (nor->bouncebuf[0] & SR2_QUAD_EN_BIT1)
return 0;
- sr_cr[1] |= CR_QUAD_EN_SPAN;
-
- /* Keep the current value of the Status Register. */
- ret = spi_nor_read_sr(nor, &sr_cr[0]);
- if (ret)
- return ret;
-
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- return ret;
-
- /* Read back and check it. */
- ret = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- return ret;
-
- if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
- dev_err(nor->dev, "Spansion Quad bit not set\n");
- return -EIO;
- }
-
- return 0;
+ return spi_nor_write_16bit_cr_and_check(nor, nor->bouncebuf[0]);
}
/**
@@ -2112,7 +2114,7 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
*
* Read-modify-write function that clears the Block Protection bits from the
* Status Register without affecting other bits. The function is tightly
- * coupled with the spansion_read_cr_quad_enable() function. Both assume that
+ * coupled with the spi_nor_sr2_bit1_quad_enable() function. Both assume that
* the Write Register with 16 bits, together with the Read Configuration
* Register (35h) instructions are supported.
*
@@ -2133,7 +2135,7 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
* When the configuration register Quad Enable bit is one, only the
* Write Status (01h) command with two data bytes may be used.
*/
- if (sr_cr[1] & CR_QUAD_EN_SPAN) {
+ if (sr_cr[1] & SR2_QUAD_EN_BIT1) {
ret = spi_nor_read_sr(nor, &sr_cr[0]);
if (ret)
return ret;
@@ -3637,7 +3639,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
* supported.
*/
nor->flags |= SNOR_F_NO_READ_CR;
- flash->quad_enable = spansion_no_read_cr_quad_enable;
+ flash->quad_enable = spi_nor_sr2_bit1_quad_enable;
break;
case BFPT_DWORD15_QER_SR1_BIT6:
@@ -3658,7 +3660,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
* assumption of a 16-bit Write Status (01h) command.
*/
nor->flags |= SNOR_F_HAS_16BIT_SR;
- flash->quad_enable = spansion_read_cr_quad_enable;
+ flash->quad_enable = spi_nor_sr2_bit1_quad_enable;
break;
default:
@@ -4621,7 +4623,7 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
u8 i, erase_mask;
/* Initialize legacy flash parameters and settings. */
- flash->quad_enable = spansion_read_cr_quad_enable;
+ flash->quad_enable = spi_nor_sr2_bit1_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;
/* Default to 16-bit Write Status (01h) Command */
@@ -4839,7 +4841,7 @@ static int spi_nor_init(struct spi_nor *nor)
int err;
if (nor->clear_sr_bp) {
- if (nor->flash.quad_enable == spansion_read_cr_quad_enable)
+ if (nor->flash.quad_enable == spi_nor_sr2_bit1_quad_enable)
nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
err = nor->clear_sr_bp(nor);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 3a835de90b6a..5590a36eb43e 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -144,10 +144,8 @@
#define FSR_P_ERR BIT(4) /* Program operation status */
#define FSR_PT_ERR BIT(1) /* Protection error bit */
-/* Configuration Register bits. */
-#define CR_QUAD_EN_SPAN BIT(1) /* Spansion Quad I/O */
-
/* Status Register 2 bits. */
+#define SR2_QUAD_EN_BIT1 BIT(1)
#define SR2_QUAD_EN_BIT7 BIT(7)
/* Supported SPI protocols */
--
2.9.5
_______________________________________________
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 v2 21/22] mtd: spi-nor: Update sr2_bit7_quad_enable()
From: Tudor.Ambarus @ 2019-09-24 7:47 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Rename the method to spi_nor_sr2_bit7_quad_enable(). Do the
read back test on all the eight bits of the Status Register,
not just the QE one.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8fd1c04f75d9..a53e2cdc564c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2038,7 +2038,7 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
}
/**
- * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
+ * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
* @nor: pointer to a 'struct spi_nor'
*
* Set the Quad Enable (QE) bit in the Status Register 2.
@@ -2049,10 +2049,11 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
*
* Return: 0 on success, -errno otherwise.
*/
-static int sr2_bit7_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
{
u8 *sr2 = nor->bouncebuf;
int ret;
+ u8 sr2_written;
/* Check current Quad Enable bit value. */
ret = spi_nor_read_sr2(nor, sr2);
@@ -2069,13 +2070,15 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
if (ret)
return ret;
+ sr2_written = *sr2;
+
/* Read back and check it. */
ret = spi_nor_read_sr2(nor, sr2);
if (ret)
return ret;
- if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
- dev_err(nor->dev, "SR2 Quad bit not set\n");
+ if (*sr2 != sr2_written) {
+ dev_err(nor->dev, "Read back test failed\n");
return -EIO;
}
@@ -3649,7 +3652,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
case BFPT_DWORD15_QER_SR2_BIT7:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- flash->quad_enable = sr2_bit7_quad_enable;
+ flash->quad_enable = spi_nor_sr2_bit7_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
--
2.9.5
_______________________________________________
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 v2 19/22] mtd: spi-nor: Rework macronix_quad_enable()
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Rename method to a generic name: spi_nor_sr1_bit6_quad_enable().
Use spi_nor_write_sr1_and_check(). Now we check the validity of all
the eight bits of the Status Register, not just of the SR1_QUAD_EN_BIT6.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 34 ++++++++++------------------------
include/linux/mtd/spi-nor.h | 2 +-
2 files changed, 11 insertions(+), 25 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8ada2003f1c9..112f93cec7ba 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1933,16 +1933,15 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
}
/**
- * macronix_quad_enable() - set QE bit in Status Register.
+ * spi_nor_sr1_bit6_quad_enable() - Set the Quad Enable BIT(6) in the Status
+ * Register 1.
* @nor: pointer to a 'struct spi_nor'
*
- * Set the Quad Enable (QE) bit in the Status Register.
- *
- * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
+ * Bit 6 of the Status Register 1 is the QE bit for Macronix like QSPI memories.
*
* Return: 0 on success, -errno otherwise.
*/
-static int macronix_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr1_bit6_quad_enable(struct spi_nor *nor)
{
int ret;
@@ -1950,25 +1949,12 @@ static int macronix_quad_enable(struct spi_nor *nor)
if (ret)
return ret;
- if (nor->bouncebuf[0] & SR_QUAD_EN_MX)
+ if (nor->bouncebuf[0] & SR1_QUAD_EN_BIT6)
return 0;
- nor->bouncebuf[0] |= SR_QUAD_EN_MX;
+ nor->bouncebuf[0] |= SR1_QUAD_EN_BIT6;
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
- dev_err(nor->dev, "Macronix Quad bit not set\n");
- return -EIO;
- }
-
- return 0;
+ return spi_nor_write_sr1_and_check(nor, nor->bouncebuf[0]);
}
/**
@@ -2272,7 +2258,7 @@ static void gd25q256_default_init(struct spi_nor *nor)
* indicate the quad_enable method for this case, we need
* to set it in the default_init fixup hook.
*/
- nor->flash.quad_enable = macronix_quad_enable;
+ nor->flash.quad_enable = spi_nor_sr1_bit6_quad_enable;
}
static struct spi_nor_fixups gd25q256_fixups = {
@@ -3656,7 +3642,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
case BFPT_DWORD15_QER_SR1_BIT6:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- flash->quad_enable = macronix_quad_enable;
+ flash->quad_enable = spi_nor_sr1_bit6_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT7:
@@ -4553,7 +4539,7 @@ static int spi_nor_setup(struct spi_nor *nor,
static void macronix_set_default_init(struct spi_nor *nor)
{
- nor->flash.quad_enable = macronix_quad_enable;
+ nor->flash.quad_enable = spi_nor_sr1_bit6_quad_enable;
nor->flash.set_4byte = macronix_set_4byte;
}
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fc3a8f5209f0..3a835de90b6a 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -133,7 +133,7 @@
#define SR_E_ERR BIT(5)
#define SR_P_ERR BIT(6)
-#define SR_QUAD_EN_MX BIT(6) /* Macronix Quad I/O */
+#define SR1_QUAD_EN_BIT6 BIT(6)
/* Enhanced Volatile Configuration Register bits */
#define EVCR_QUAD_EN_MICRON BIT(7) /* Micron Quad I/O */
--
2.9.5
_______________________________________________
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 v2 18/22] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Make sure that when doing a lock() or an unlock() operation we don't clear
the QE bit from Status Register 2.
JESD216 revB or later offers information about the *default* Status
Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
standard, Status Register 1 refers to the first data byte transferred on a
Read Status (05h) or Write Status (01h) command. Status register 2 refers
to the byte read using instruction 35h. Status register 2 is the second
byte transferred in a Write Status (01h) command.
Industry naming and definitions of these Status Registers may differ.
The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
There are cases in which writing only one byte to the Status Register 1
has the side-effect of clearing Status Register 2 and implicitly the Quad
Enable bit. This side-effect is hit just by the
BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.
Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 165 +++++++++++++++++++++++++++++++++++++-----
include/linux/mtd/spi-nor.h | 2 +
2 files changed, 147 insertions(+), 20 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 48bcb2ee1be5..8ada2003f1c9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -836,6 +836,127 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
return ret;
}
+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
+ * ensure that the byte written match the received value.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+
+ nor->bouncebuf[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
+ if (ret)
+ return ret;
+
+ if (nor->bouncebuf[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
+ * Status Register 2 in one shot. Ensure that the byte written in the Status
+ * Register 1 match the received value, and that the 16-bit Write did not
+ * affect what was already in the Status Register 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+ u8 cr_written;
+
+ /* Make sure we don't overwrite the contents of Status Register 2. */
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+ } else if (nor->flash.quad_enable) {
+ /*
+ * If the Status Register 2 Read command (35h) is not
+ * supported, we should at least be sure we don't
+ * change the value of the SR2 Quad Enable bit.
+ *
+ * We can safely assume that when the Quad Enable method is
+ * set, the value of the QE bit is one, as a consequence of the
+ * nor->flash.quad_enable() call.
+ *
+ * We can safely assume that the Quad Enable bit is present in
+ * the Status Register 2 at BIT(1). According to the JESD216
+ * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
+ * Write Status (01h) command is available just for the cases
+ * in which the QE bit is described in SR2 at BIT(1).
+ */
+ sr_cr[1] = CR_QUAD_EN_SPAN;
+ } else {
+ sr_cr[1] = 0;
+ }
+
+ sr_cr[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, sr_cr, 2);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &sr_cr[0]);
+ if (ret)
+ return ret;
+
+ if (sr_cr[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ cr_written = sr_cr[1];
+
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (cr_written != sr_cr[1]) {
+ dev_err(nor->dev, "CR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value without affecting other bits in the
+ * Status Register 1 and 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_16bit_sr_and_check(nor, sr1);
+
+ return spi_nor_write_sr1_and_check(nor, sr1);
+}
+
static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
@@ -1492,24 +1613,6 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
return ret;
}
-/* Write status register and ensure bits in mask match written values */
-static int write_sr_and_check(struct spi_nor *nor, u8 status_new)
-{
- int ret;
-
- nor->bouncebuf[0] = status_new;
-
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
-}
-
static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
uint64_t *len)
{
@@ -1673,7 +1776,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) < (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
@@ -1758,7 +1861,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) > (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new);
+ return spi_nor_write_sr_and_check(nor, status_new);
}
/*
@@ -3536,19 +3639,39 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
break;
case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
+ /*
+ * Writing only one byte to the Status Register has the
+ * side-effect of clearing Status Register 2.
+ */
+ /* fall through */
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
+ /*
+ * Read Configuration Register (35h) instruction is not
+ * supported.
+ */
+ nor->flags |= SNOR_F_NO_READ_CR;
flash->quad_enable = spansion_no_read_cr_quad_enable;
break;
case BFPT_DWORD15_QER_SR1_BIT6:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = macronix_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT7:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = sr2_bit7_quad_enable;
break;
case BFPT_DWORD15_QER_SR2_BIT1:
+ /*
+ * JESD216 rev B or later does not specify if writing only one
+ * byte to the Status Register clears or not the Status
+ * Register 2, so let's be cautious and keep the default
+ * assumption of a 16-bit Write Status (01h) command.
+ */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
flash->quad_enable = spansion_read_cr_quad_enable;
break;
@@ -4515,6 +4638,8 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
flash->quad_enable = spansion_read_cr_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;
+ /* Default to 16-bit Write Status (01h) Command */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
/* Set SPI NOR sizes. */
flash->size = (u64)info->sector_size * info->n_sectors;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 12961b157743..fc3a8f5209f0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,8 @@ enum spi_nor_option_flags {
SNOR_F_4B_OPCODES = BIT(6),
SNOR_F_HAS_4BAIT = BIT(7),
SNOR_F_HAS_LOCK = BIT(8),
+ SNOR_F_HAS_16BIT_SR = BIT(9),
+ SNOR_F_NO_READ_CR = BIT(10),
};
/**
--
2.9.5
_______________________________________________
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 v2 17/22] mtd: spi-nor: Check all the bits written, not just the BP ones
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Check that all the bits written in the write_sr_and_check() method
match the status_new received value. Failing to write the other bits
is dangerous too, extend the check.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 6429c855547e..48bcb2ee1be5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1493,7 +1493,7 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
}
/* Write status register and ensure bits in mask match written values */
-static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
+static int write_sr_and_check(struct spi_nor *nor, u8 status_new)
{
int ret;
@@ -1507,7 +1507,7 @@ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
if (ret)
return ret;
- return ((nor->bouncebuf[0] & mask) != (status_new & mask)) ? -EIO : 0;
+ return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
}
static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
@@ -1673,7 +1673,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) < (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new, mask);
+ return write_sr_and_check(nor, status_new);
}
/*
@@ -1758,7 +1758,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) > (status_old & mask))
return -EINVAL;
- return write_sr_and_check(nor, status_new, mask);
+ return write_sr_and_check(nor, status_new);
}
/*
--
2.9.5
_______________________________________________
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 v2 16/22] mtd: spi-nor: Fix errno on quad_enable methods
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
When the Read-Modify-Write-Read-Back Quad Enable methods failed on
the Read-Back, they returned -EINVAL. Since this is an I/O error,
return -EIO.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 668afa9a8c87..6429c855547e 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1862,7 +1862,7 @@ static int macronix_quad_enable(struct spi_nor *nor)
if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
dev_err(nor->dev, "Macronix Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}
return 0;
@@ -1940,7 +1940,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
dev_err(nor->dev, "Spansion Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}
return 0;
@@ -1985,7 +1985,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
dev_err(nor->dev, "SR2 Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}
return 0;
--
2.9.5
_______________________________________________
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 v2 15/22] mtd: spi-nor: Drop spansion_quad_enable()
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
Drop the default spansion_quad_enable() method and replace it with
spansion_read_cr_quad_enable().
The function was buggy, it didn't care about the previous values
of the Status and Configuration Registers. spansion_read_cr_quad_enable()
is a Read-Modify-Write-Check function that keeps track of what were
the previous values of the Status and Configuration Registers.
In terms of instruction types sent to the flash, the only difference
between the spansion_quad_enable() and spansion_read_cr_quad_enable()
is that the later calls spi_nor_read_sr(). We can safely assume that all
flashes support spi_nor_read_sr(), because all flashes call it in
spi_nor_sr_ready(). The transition from spansion_quad_enable() to
spansion_read_cr_quad_enable() will not affect anybody, drop the buggy
code.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 58 ++++---------------------------------------
1 file changed, 5 insertions(+), 53 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d971f5a4b11f..668afa9a8c87 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1869,54 +1869,6 @@ static int macronix_quad_enable(struct spi_nor *nor)
}
/**
- * spansion_quad_enable() - set QE bit in Configuraiton Register.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function is kept for legacy purpose because it has been used for a
- * long time without anybody complaining but it should be considered as
- * deprecated and maybe buggy.
- * First, this function doesn't care about the previous values of the Status
- * and Configuration Registers when it sets the QE bit (bit 1) in the
- * Configuration Register: all other bits are cleared, which may have unwanted
- * side effects like removing some block protections.
- * Secondly, it uses the Read Configuration Register (35h) instruction though
- * some very old and few memories don't support this instruction. If a pull-up
- * resistor is present on the MISO/IO1 line, we might still be able to pass the
- * "read back" test because the QSPI memory doesn't recognize the command,
- * so leaves the MISO/IO1 line state unchanged, hence spi_nor_read_cr(nor, cr)
- * gets the 0xFF value.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_quad_enable(struct spi_nor *nor)
-{
- u8 *sr_cr = nor->bouncebuf;
- int ret;
-
- sr_cr[0] = 0;
- sr_cr[1] = CR_QUAD_EN_SPAN;
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- return ret;
-
- /* read back and check it */
- ret = spi_nor_read_cr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
- dev_err(nor->dev, "Spansion Quad bit not set\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-/**
* spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
* @nor: pointer to a 'struct spi_nor'
*
@@ -2071,9 +2023,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *nor)
*
* Read-modify-write function that clears the Block Protection bits from the
* Status Register without affecting other bits. The function is tightly
- * coupled with the spansion_quad_enable() function. Both assume that the Write
- * Register with 16 bits, together with the Read Configuration Register (35h)
- * instructions are supported.
+ * coupled with the spansion_read_cr_quad_enable() function. Both assume that
+ * the Write Register with 16 bits, together with the Read Configuration
+ * Register (35h) instructions are supported.
*
* Return: 0 on success, -errno otherwise.
*/
@@ -4560,7 +4512,7 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
u8 i, erase_mask;
/* Initialize legacy flash parameters and settings. */
- flash->quad_enable = spansion_quad_enable;
+ flash->quad_enable = spansion_read_cr_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;
@@ -4776,7 +4728,7 @@ static int spi_nor_init(struct spi_nor *nor)
int err;
if (nor->clear_sr_bp) {
- if (nor->flash.quad_enable == spansion_quad_enable)
+ if (nor->flash.quad_enable == spansion_read_cr_quad_enable)
nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
err = nor->clear_sr_bp(nor);
--
2.9.5
_______________________________________________
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 v2 13/22] mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
From: Tudor.Ambarus @ 2019-09-24 7:46 UTC (permalink / raw)
To: vigneshr, boris.brezillon, marek.vasut, linux-mtd, geert+renesas,
jonas
Cc: linux-aspeed, Tudor.Ambarus, andrew, richard, linux-kernel, vz,
linux-mediatek, joel, miquel.raynal, matthias.bgg,
computersforpeace, dwmw2, linux-arm-kernel
In-Reply-To: <20190924074533.6618-1-tudor.ambarus@microchip.com>
From: Tudor Ambarus <tudor.ambarus@microchip.com>
spi_nor_clear_sr() and spi_nor_clear_fsr() are called just in case
of errors. The callers didn't check their return value. Make them
of type void and print an error in case the operations fail.
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
drivers/mtd/spi-nor/spi-nor.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b7f86f518331..191a76c3f7bb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -578,8 +578,16 @@ static int s3an_sr_ready(struct spi_nor *nor)
return !!(nor->bouncebuf[0] & XSR_RDY);
}
-static int spi_nor_clear_sr(struct spi_nor *nor)
+/**
+ * spi_nor_clear_sr() - Clear the Status Register 1.
+ * @nor: pointer to 'struct spi_nor'
+ *
+ * Prints error in case the operation fails.
+ */
+static void spi_nor_clear_sr(struct spi_nor *nor)
{
+ int ret;
+
if (nor->spimem) {
struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLSR, 1),
@@ -587,10 +595,14 @@ static int spi_nor_clear_sr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
- return spi_mem_exec_op(nor->spimem, &op);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR,
+ NULL, 0);
}
- return nor->controller_ops->write_reg(nor, SPINOR_OP_CLSR, NULL, 0);
+ if (ret)
+ dev_err(nor->dev, "error %d clearing Status Register 1\n", ret);
}
static int spi_nor_sr_ready(struct spi_nor *nor)
@@ -614,8 +626,16 @@ static int spi_nor_sr_ready(struct spi_nor *nor)
return !(nor->bouncebuf[0] & SR_WIP);
}
-static int spi_nor_clear_fsr(struct spi_nor *nor)
+/**
+ * spi_nor_clear_fsr() - Clear the Flag Status Register.
+ * @nor: pointer to 'struct spi_nor'
+ *
+ * Prints error in case the operation fails.
+ */
+static void spi_nor_clear_fsr(struct spi_nor *nor)
{
+ int ret;
+
if (nor->spimem) {
struct spi_mem_op op =
SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_CLFSR, 1),
@@ -623,10 +643,14 @@ static int spi_nor_clear_fsr(struct spi_nor *nor)
SPI_MEM_OP_NO_DUMMY,
SPI_MEM_OP_NO_DATA);
- return spi_mem_exec_op(nor->spimem, &op);
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR,
+ NULL, 0);
}
- return nor->controller_ops->write_reg(nor, SPINOR_OP_CLFSR, NULL, 0);
+ if (ret)
+ dev_err(nor->dev, "error %d clearing FSR\n", ret);
}
static int spi_nor_fsr_ready(struct spi_nor *nor)
--
2.9.5
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox