[PATCH v2] power: supply: register HWMON devices with valid names
With the introduction of the HWMON compatibility layer to the power
supply framework in Linux 5.3, all power supply devices' names can be
used directly to create HWMON devices with the same names.
But HWMON has rules on allowable names that are different from those
used in the power supply framework. The dash character is forbidden, as
it is used by the libsensors library in userspace as a separator,
whereas this character is used in the device names in more than half of
the existing power supply drivers. This last case is consistent with the
typical naming usage with MFD and Device Tree.
This leads to warnings in the kernel log, with the format:
power_supply gpio-charger: hwmon: \
'gpio-charger' is not a valid name attribute, please fix
Add a protection to power_supply_add_hwmon_sysfs() that replaces any
dash in the device name with an underscore when registering with the
HWMON framework. Other forbidden characters (star, slash, space, tab,
newline) are not replaced, as they are not in common use.
Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer")
Signed-off-by: Romain Izard
Reviewed-by: Guenter Roeck
---
v2: Remove a superfluous cast
drivers/power/supply/power_supply_hwmon.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_hwmon.c
b/drivers/power/supply/power_supply_hwmon.c
index 51fe60440d12..75cf861ba492 100644
--- a/drivers/power/supply/power_supply_hwmon.c
+++ b/drivers/power/supply/power_supply_hwmon.c
@@ -284,6 +284,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
struct device *dev = >dev;
struct device *hwmon;
int ret, i;
+ const char *name;
if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
GFP_KERNEL))
@@ -334,7 +335,19 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
}
}
- hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
+ name = psy->desc->name;
+ if (strchr(name, '-')) {
+ char *new_name;
+
+ new_name = devm_kstrdup(dev, name, GFP_KERNEL);
+ if (!new_name) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ strreplace(new_name, '-', '_');
+ name = new_name;
+ }
+ hwmon = devm_hwmon_device_register_with_info(dev, name,
psyhw,
_supply_hwmon_chip_info,
NULL);
--
2.17.1
Re: [PATCH] power: supply: register HWMON devices with valid names
On Thu, Aug 22, 2019 at 09:12:07AM -0700, Guenter Roeck wrote:
> On Thu, Aug 22, 2019 at 05:09:19PM +0200, Romain Izard wrote:
> > With the introduction of the HWMON compatibility layer to the power
> > supply framework in Linux 5.3, all power supply devices' names can be
> > used directly to create HWMON devices with the same names.
> >
> > But HWMON has rules on allowable names that are different from the power
> > supply framework. The dash character is forbidden, as it is used by the
> > libsensors library in userspace as a separator, whereas this character
> > is used in the device names in more than half of the existing power
> > supply drivers. This last case is consistent with the typical naming
> > usage with MFD and Device Tree.
> >
> > This leads to warnings in the kernel log, with the format:
> >
> > power_supply gpio-charger: hwmon: \
> > 'gpio-charger' is not a valid name attribute, please fix
> >
> > Add a protection to power_supply_add_hwmon_sysfs() that replaces any
> > dash in the device name with an underscore when registering with the
> > HWMON framework. Other forbidden characters (star, slash, space, tab,
> > newline) are not replaced, as they are not in common use.
> >
> > Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer")
> > Signed-off-by: Romain Izard
> > ---
> > drivers/power/supply/power_supply_hwmon.c | 15 ++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/power/supply/power_supply_hwmon.c
> > b/drivers/power/supply/power_supply_hwmon.c
> > index 51fe60440d12..ebe964bd512c 100644
> > --- a/drivers/power/supply/power_supply_hwmon.c
> > +++ b/drivers/power/supply/power_supply_hwmon.c
> > @@ -284,6 +284,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply
> > *psy)
> > struct device *dev = >dev;
> > struct device *hwmon;
> > int ret, i;
> > + const char *name;
> >
> > if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
> >GFP_KERNEL))
> > @@ -334,7 +335,19 @@ int power_supply_add_hwmon_sysfs(struct power_supply
> > *psy)
> > }
> > }
> >
> > - hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
> > + name = psy->desc->name;
> > + if (strchr(name, '-')) {
> > + char *new_name;
> > +
> > + new_name = devm_kstrdup(dev, name, GFP_KERNEL);
> > + if (!new_name) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > + strreplace(new_name, '-', '_');
> > + name = (const char *) new_name;
>
> Is that typecast necessary ?
Indeed, it is not. I'll remove it if a V2 is necessary.
>
> Other than that,
>
> Reviewed-by: Guenter Roeck
>
> > + }
> > + hwmon = devm_hwmon_device_register_with_info(dev, name,
> > psyhw,
> > _supply_hwmon_chip_info,
> > NULL);
Thanks for your review.
Best regards,
--
Romain Izard
[PATCH] power: supply: register HWMON devices with valid names
With the introduction of the HWMON compatibility layer to the power
supply framework in Linux 5.3, all power supply devices' names can be
used directly to create HWMON devices with the same names.
But HWMON has rules on allowable names that are different from the power
supply framework. The dash character is forbidden, as it is used by the
libsensors library in userspace as a separator, whereas this character
is used in the device names in more than half of the existing power
supply drivers. This last case is consistent with the typical naming
usage with MFD and Device Tree.
This leads to warnings in the kernel log, with the format:
power_supply gpio-charger: hwmon: \
'gpio-charger' is not a valid name attribute, please fix
Add a protection to power_supply_add_hwmon_sysfs() that replaces any
dash in the device name with an underscore when registering with the
HWMON framework. Other forbidden characters (star, slash, space, tab,
newline) are not replaced, as they are not in common use.
Fixes: e67d4dfc9ff1 ("power: supply: Add HWMON compatibility layer")
Signed-off-by: Romain Izard
---
drivers/power/supply/power_supply_hwmon.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/power_supply_hwmon.c
b/drivers/power/supply/power_supply_hwmon.c
index 51fe60440d12..ebe964bd512c 100644
--- a/drivers/power/supply/power_supply_hwmon.c
+++ b/drivers/power/supply/power_supply_hwmon.c
@@ -284,6 +284,7 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
struct device *dev = >dev;
struct device *hwmon;
int ret, i;
+ const char *name;
if (!devres_open_group(dev, power_supply_add_hwmon_sysfs,
GFP_KERNEL))
@@ -334,7 +335,19 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
}
}
- hwmon = devm_hwmon_device_register_with_info(dev, psy->desc->name,
+ name = psy->desc->name;
+ if (strchr(name, '-')) {
+ char *new_name;
+
+ new_name = devm_kstrdup(dev, name, GFP_KERNEL);
+ if (!new_name) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ strreplace(new_name, '-', '_');
+ name = (const char *) new_name;
+ }
+ hwmon = devm_hwmon_device_register_with_info(dev, name,
psyhw,
_supply_hwmon_chip_info,
NULL);
--
2.17.1
Re: A potential broken at platform driver?
On Mon, Jun 03, 2019 at 08:02:55PM +0200, Greg KH wrote:
> > @@ -394,7 +432,7 @@ static struct platform_driver stratix10_rsu_driver = {
> > .remove = stratix10_rsu_remove,
> > .driver = {
> > .name = "stratix10-rsu",
> > - .groups = rsu_groups,
> > +// .groups = rsu_groups,
>
> Are you sure this is the correct pointer? I think that might be
> pointing to the driver's attributes, not the device's attributes.
>
> If platform drivers do not have a way to register groups properly, then
> that really needs to be fixed, as trying to register it by yourself as
> you are doing, is ripe for racing with userspace.
This is a very common issue with platform drivers, and it seems to me that
it is not possible to add device attributes when binding a device to a
driver without entering the race condition.
My understanding is the following one:
The root cause is that the device has already been created and reported
to the userspace with a KOBJ_ADD uevent before the device and the driver
are bound together. On receiving this event, userspace will react, and
it will try to read the device's attributes. In parallel the kernel will
try to find a matching driver. If a driver is found, the kernel will
call the probe function from the driver with the device as a parameter,
and if successful a KOBJ_BIND uevent will be sent to userspace, but this
is a recent addition.
Unfortunately, not all created devices will be bound to a driver, and the
existing udev code relies on KOBJ_ADD uevents rather than KOBJ_BIND uevents.
If new per-device attributes have been added to the device during the
binding stage userspace may or may not see them, depending on when userspace
tries to read the device's attributes.
I have this possible workaround, but I do not know if it is a good solution:
When binding the device and the driver together, create a new device as a
child to the current device, and fill its "groups" member to point to the
per-device attributes' group. As the device will be created with all the
attributes, it will not be affected by the race issues. The functions
handling the attributes will need to be modified to use the parents of their
"device" parameter, instead of the device itself. Additionnaly, the sysfs
location of the attributes will be different, as the child device will show
up in the sysfs path. But for a newly introduced device this will not be
a problem.
Is this a good compromise ?
Best regards,
--
Romain Izard
Re: [PATCH v2 9/9] misc: atmel_tclib: do not probe already used TCBs
On Mon, Apr 15, 2019 at 05:08:56PM +0200, Alexandre Belloni wrote: > The TCBs that have children are using the proper DT bindings and don't need > to be handled by tclib. > > Cc: Arnd Bergmann > Signed-off-by: Alexandre Belloni > --- > drivers/misc/atmel_tclib.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/misc/atmel_tclib.c b/drivers/misc/atmel_tclib.c > index 194f774ab3a1..c1f5aba1c6f2 100644 > --- a/drivers/misc/atmel_tclib.c > +++ b/drivers/misc/atmel_tclib.c > @@ -111,6 +111,9 @@ static int __init tc_probe(struct platform_device *pdev) > struct resource *r; > unsigned inti; > > + if (of_get_child_count(pdev->dev.of_node)) > + return 0; > + > irq = platform_get_irq(pdev, 0); > if (irq < 0) > return -EINVAL; This lead to a panic during shutdown, as tc_shutdown is called on the device that has not been initialized and it has not been designed for this. -- Romain Izard
Re: [PATCH] ARM: dts: at91: sama5d2: use the divided clock for SMC
Le mar. 20 nov. 2018 à 18:16, Alexandre Belloni
a écrit :
>
> Hello Romain,
>
> On 20/11/2018 17:57:37+0100, Romain Izard wrote:
> > The SAMA5D2 is different from SAMA5D3 and SAMA5D4, as there are two
> > different clocks for the peripherals in the SoC. The Static Memory
> > controller is connected to the divided master clock.
> >
> > Unfortunately, the device tree does not correctly show this and uses the
> > master clock directly. This clock is then used by the code for the NAND
> > controller to calculate the timings for the controller, and we end up with
> > slow NAND Flash access.
> >
> > Fix the device tree, and the performance of Flash access is improved.
> >
> > Signed-off-by: Romain Izard
> > ---
> > arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> > index 61f68e5c48e9..b405992eb601 100644
> > --- a/arch/arm/boot/dts/sama5d2.dtsi
> > +++ b/arch/arm/boot/dts/sama5d2.dtsi
> > @@ -308,7 +308,7 @@
> > 0x1 0x0 0x6000 0x1000
> > 0x2 0x0 0x7000 0x1000
> > 0x3 0x0 0x8000 0x1000>;
> > - clocks = <>;
> > + clocks = <>;
>
> You will have to rebase on top of at91-dt. And if I'm not mistaken, this
> line should be:
>
> + clocks = < PMC_TYPE_CORE PMC_MCK2>;
>
> > status = "disabled";
> >
> > nand_controller: nand-controller {
I guess you're right but this will only reach mainline in 4.21. I get slow
flash access with 4.19 as well...
After a second look, it looks like the SAMA5D4 is affected too.
Best regards,
--
Romain Izard
Re: [PATCH] ARM: dts: at91: sama5d2: use the divided clock for SMC
Le mar. 20 nov. 2018 à 18:16, Alexandre Belloni
a écrit :
>
> Hello Romain,
>
> On 20/11/2018 17:57:37+0100, Romain Izard wrote:
> > The SAMA5D2 is different from SAMA5D3 and SAMA5D4, as there are two
> > different clocks for the peripherals in the SoC. The Static Memory
> > controller is connected to the divided master clock.
> >
> > Unfortunately, the device tree does not correctly show this and uses the
> > master clock directly. This clock is then used by the code for the NAND
> > controller to calculate the timings for the controller, and we end up with
> > slow NAND Flash access.
> >
> > Fix the device tree, and the performance of Flash access is improved.
> >
> > Signed-off-by: Romain Izard
> > ---
> > arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> > index 61f68e5c48e9..b405992eb601 100644
> > --- a/arch/arm/boot/dts/sama5d2.dtsi
> > +++ b/arch/arm/boot/dts/sama5d2.dtsi
> > @@ -308,7 +308,7 @@
> > 0x1 0x0 0x6000 0x1000
> > 0x2 0x0 0x7000 0x1000
> > 0x3 0x0 0x8000 0x1000>;
> > - clocks = <>;
> > + clocks = <>;
>
> You will have to rebase on top of at91-dt. And if I'm not mistaken, this
> line should be:
>
> + clocks = < PMC_TYPE_CORE PMC_MCK2>;
>
> > status = "disabled";
> >
> > nand_controller: nand-controller {
I guess you're right but this will only reach mainline in 4.21. I get slow
flash access with 4.19 as well...
After a second look, it looks like the SAMA5D4 is affected too.
Best regards,
--
Romain Izard
[PATCH] ARM: dts: at91: sama5d2: use the divided clock for SMC
The SAMA5D2 is different from SAMA5D3 and SAMA5D4, as there are two
different clocks for the peripherals in the SoC. The Static Memory
controller is connected to the divided master clock.
Unfortunately, the device tree does not correctly show this and uses the
master clock directly. This clock is then used by the code for the NAND
controller to calculate the timings for the controller, and we end up with
slow NAND Flash access.
Fix the device tree, and the performance of Flash access is improved.
Signed-off-by: Romain Izard
---
arch/arm/boot/dts/sama5d2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 61f68e5c48e9..b405992eb601 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -308,7 +308,7 @@
0x1 0x0 0x6000 0x1000
0x2 0x0 0x7000 0x1000
0x3 0x0 0x8000 0x1000>;
- clocks = <>;
+ clocks = <>;
status = "disabled";
nand_controller: nand-controller {
--
2.17.1
[PATCH] ARM: dts: at91: sama5d2: use the divided clock for SMC
The SAMA5D2 is different from SAMA5D3 and SAMA5D4, as there are two
different clocks for the peripherals in the SoC. The Static Memory
controller is connected to the divided master clock.
Unfortunately, the device tree does not correctly show this and uses the
master clock directly. This clock is then used by the code for the NAND
controller to calculate the timings for the controller, and we end up with
slow NAND Flash access.
Fix the device tree, and the performance of Flash access is improved.
Signed-off-by: Romain Izard
---
arch/arm/boot/dts/sama5d2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 61f68e5c48e9..b405992eb601 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -308,7 +308,7 @@
0x1 0x0 0x6000 0x1000
0x2 0x0 0x7000 0x1000
0x3 0x0 0x8000 0x1000>;
- clocks = <>;
+ clocks = <>;
status = "disabled";
nand_controller: nand-controller {
--
2.17.1
Re: [PATCH 2/2] watchdog: sama5d4: write the mode register in two steps
2018-09-14 12:27 GMT+02:00 Alexandre Belloni :
> On 14/09/2018 12:13:39+0200, Romain Izard wrote:
>> The specification for SAMA5D2 and SAMA5D4 chips, that use this IP for
>> their watchdog timer, has the following advice regarding the Mode Register:
>>
>> "When setting the WDDIS bit, and while it is set, the fields WDV and WDD
>> must not be modified."
>>
>> I have observed on a board based on a SAMA5D2 chip that using any other
>> timeout duration than the default 16s in the device tree will reset the
>> board when the watchdog device is opened; this is probably due to ignoring
>> the aforementioned constraint.
>>
>> To fix this, read the content of the Mode Register before writing it,
>> and split the access into two parts if WDV or WDD need to be changed.
>>
>
> Hum, that is really weird because when I developed
> 015b528644a84b0018d3286ecd6ea5f82dce0180, I tested with a program doing:
>
> flags = WDIOS_DISABLECARD;
> ioctl(fd, WDIOC_SETOPTIONS, );
> for (i = 16; i > 2; i--) {
> ioctl(fd, WDIOC_SETTIMEOUT, );
> }
>
> ioctl(fd, WDIOC_KEEPALIVE, );
>
> flags = WDIOS_ENABLECARD;
> ioctl(fd, WDIOC_SETOPTIONS, );
>
> for (i = 16; i > 2; i--) {
> ioctl(fd, WDIOC_SETTIMEOUT, );
> }
>
> This would immediately reproduce the reset when changing WDV/WDD with
> WDDIS set.
>
> I'll test again.
>
The issue is visible when setting a custom value for the timeout on startup.
In the past it was only possible to do so with a module parameter, and the
previous patch in the series makes it possible to do with the device tree.
When using the Linux4SAM 5.7 release, it is sufficient to set the timeout on
the command line to reproduce the issue:
In the bootloader:
# setenv bootargs $bootargs sama5d4_wdt.wdt_timeout=10
To trigger an immediate reset (with some code that should work):
# (echo 1; while sleep 3; do echo 1; done) > /dev/watchdog
Best regards,
--
Romain Izard
Re: [PATCH 2/2] watchdog: sama5d4: write the mode register in two steps
2018-09-14 12:27 GMT+02:00 Alexandre Belloni :
> On 14/09/2018 12:13:39+0200, Romain Izard wrote:
>> The specification for SAMA5D2 and SAMA5D4 chips, that use this IP for
>> their watchdog timer, has the following advice regarding the Mode Register:
>>
>> "When setting the WDDIS bit, and while it is set, the fields WDV and WDD
>> must not be modified."
>>
>> I have observed on a board based on a SAMA5D2 chip that using any other
>> timeout duration than the default 16s in the device tree will reset the
>> board when the watchdog device is opened; this is probably due to ignoring
>> the aforementioned constraint.
>>
>> To fix this, read the content of the Mode Register before writing it,
>> and split the access into two parts if WDV or WDD need to be changed.
>>
>
> Hum, that is really weird because when I developed
> 015b528644a84b0018d3286ecd6ea5f82dce0180, I tested with a program doing:
>
> flags = WDIOS_DISABLECARD;
> ioctl(fd, WDIOC_SETOPTIONS, );
> for (i = 16; i > 2; i--) {
> ioctl(fd, WDIOC_SETTIMEOUT, );
> }
>
> ioctl(fd, WDIOC_KEEPALIVE, );
>
> flags = WDIOS_ENABLECARD;
> ioctl(fd, WDIOC_SETOPTIONS, );
>
> for (i = 16; i > 2; i--) {
> ioctl(fd, WDIOC_SETTIMEOUT, );
> }
>
> This would immediately reproduce the reset when changing WDV/WDD with
> WDDIS set.
>
> I'll test again.
>
The issue is visible when setting a custom value for the timeout on startup.
In the past it was only possible to do so with a module parameter, and the
previous patch in the series makes it possible to do with the device tree.
When using the Linux4SAM 5.7 release, it is sufficient to set the timeout on
the command line to reproduce the issue:
In the bootloader:
# setenv bootargs $bootargs sama5d4_wdt.wdt_timeout=10
To trigger an immediate reset (with some code that should work):
# (echo 1; while sleep 3; do echo 1; done) > /dev/watchdog
Best regards,
--
Romain Izard
[PATCH 2/2] watchdog: sama5d4: write the mode register in two steps
The specification for SAMA5D2 and SAMA5D4 chips, that use this IP for
their watchdog timer, has the following advice regarding the Mode Register:
"When setting the WDDIS bit, and while it is set, the fields WDV and WDD
must not be modified."
I have observed on a board based on a SAMA5D2 chip that using any other
timeout duration than the default 16s in the device tree will reset the
board when the watchdog device is opened; this is probably due to ignoring
the aforementioned constraint.
To fix this, read the content of the Mode Register before writing it,
and split the access into two parts if WDV or WDD need to be changed.
Signed-off-by: Romain Izard
---
drivers/watchdog/sama5d4_wdt.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 1e93c1b0e3cf..1e05268ad94b 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -46,7 +46,10 @@ MODULE_PARM_DESC(nowayout,
"Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-#define wdt_enabled (!(wdt->mr & AT91_WDT_WDDIS))
+#define wdt_enabled(reg) (!((reg) & AT91_WDT_WDDIS))
+
+#define wdt_different_counters(reg_a, reg_b) \
+ (((reg_a) ^ (reg_b)) & (AT91_WDT_WDV | AT91_WDT_WDD))
#define wdt_read(wdt, field) \
readl_relaxed((wdt)->reg_base + (field))
@@ -78,8 +81,11 @@ static void wdt_write_nosleep(struct sama5d4_wdt *wdt, u32
field, u32 val)
static int sama5d4_wdt_start(struct watchdog_device *wdd)
{
struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 reg = wdt_read(wdt, AT91_WDT_MR);
wdt->mr &= ~AT91_WDT_WDDIS;
+ if (!wdt_enabled(reg) && wdt_different_counters(reg, wdt->mr))
+ wdt_write(wdt, AT91_WDT_MR, reg & ~AT91_WDT_WDDIS);
wdt_write(wdt, AT91_WDT_MR, wdt->mr);
return 0;
@@ -88,8 +94,11 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd)
static int sama5d4_wdt_stop(struct watchdog_device *wdd)
{
struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 reg = wdt_read(wdt, AT91_WDT_MR);
wdt->mr |= AT91_WDT_WDDIS;
+ if (wdt_enabled(reg) && wdt_different_counters(reg, wdt->mr))
+ wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
wdt_write(wdt, AT91_WDT_MR, wdt->mr);
return 0;
@@ -122,7 +131,7 @@ static int sama5d4_wdt_set_timeout(struct watchdog_device
*wdd,
* If the watchdog is enabled, then the timeout can be updated. Else,
* wait that the user enables it.
*/
- if (wdt_enabled)
+ if (wdt_enabled(wdt->mr))
wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
wdd->timeout = timeout;
@@ -186,13 +195,17 @@ static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)
* If the watchdog is already running, we can safely update it.
* Else, we have to disable it properly.
*/
- if (wdt_enabled) {
+ reg = wdt_read(wdt, AT91_WDT_MR);
+ if (wdt_enabled(reg)) {
+ if (!wdt_enabled(wdt->mr))
+ wdt_write_nosleep(wdt, AT91_WDT_MR,
+ wdt->mr & ~AT91_WDT_WDDIS);
wdt_write_nosleep(wdt, AT91_WDT_MR, wdt->mr);
- } else {
- reg = wdt_read(wdt, AT91_WDT_MR);
- if (!(reg & AT91_WDT_WDDIS))
+ } else if (wdt_enabled(wdt->mr)) {
+ if (wdt_different_counters(reg, wdt->mr))
wdt_write_nosleep(wdt, AT91_WDT_MR,
- reg | AT91_WDT_WDDIS);
+ reg & ~AT91_WDT_WDDIS);
+ wdt_write_nosleep(wdt, AT91_WDT_MR, wdt->mr);
}
return 0;
}
--
2.17.1
[PATCH 2/2] watchdog: sama5d4: write the mode register in two steps
The specification for SAMA5D2 and SAMA5D4 chips, that use this IP for
their watchdog timer, has the following advice regarding the Mode Register:
"When setting the WDDIS bit, and while it is set, the fields WDV and WDD
must not be modified."
I have observed on a board based on a SAMA5D2 chip that using any other
timeout duration than the default 16s in the device tree will reset the
board when the watchdog device is opened; this is probably due to ignoring
the aforementioned constraint.
To fix this, read the content of the Mode Register before writing it,
and split the access into two parts if WDV or WDD need to be changed.
Signed-off-by: Romain Izard
---
drivers/watchdog/sama5d4_wdt.c | 27 ---
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 1e93c1b0e3cf..1e05268ad94b 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -46,7 +46,10 @@ MODULE_PARM_DESC(nowayout,
"Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-#define wdt_enabled (!(wdt->mr & AT91_WDT_WDDIS))
+#define wdt_enabled(reg) (!((reg) & AT91_WDT_WDDIS))
+
+#define wdt_different_counters(reg_a, reg_b) \
+ (((reg_a) ^ (reg_b)) & (AT91_WDT_WDV | AT91_WDT_WDD))
#define wdt_read(wdt, field) \
readl_relaxed((wdt)->reg_base + (field))
@@ -78,8 +81,11 @@ static void wdt_write_nosleep(struct sama5d4_wdt *wdt, u32
field, u32 val)
static int sama5d4_wdt_start(struct watchdog_device *wdd)
{
struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 reg = wdt_read(wdt, AT91_WDT_MR);
wdt->mr &= ~AT91_WDT_WDDIS;
+ if (!wdt_enabled(reg) && wdt_different_counters(reg, wdt->mr))
+ wdt_write(wdt, AT91_WDT_MR, reg & ~AT91_WDT_WDDIS);
wdt_write(wdt, AT91_WDT_MR, wdt->mr);
return 0;
@@ -88,8 +94,11 @@ static int sama5d4_wdt_start(struct watchdog_device *wdd)
static int sama5d4_wdt_stop(struct watchdog_device *wdd)
{
struct sama5d4_wdt *wdt = watchdog_get_drvdata(wdd);
+ u32 reg = wdt_read(wdt, AT91_WDT_MR);
wdt->mr |= AT91_WDT_WDDIS;
+ if (wdt_enabled(reg) && wdt_different_counters(reg, wdt->mr))
+ wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
wdt_write(wdt, AT91_WDT_MR, wdt->mr);
return 0;
@@ -122,7 +131,7 @@ static int sama5d4_wdt_set_timeout(struct watchdog_device
*wdd,
* If the watchdog is enabled, then the timeout can be updated. Else,
* wait that the user enables it.
*/
- if (wdt_enabled)
+ if (wdt_enabled(wdt->mr))
wdt_write(wdt, AT91_WDT_MR, wdt->mr & ~AT91_WDT_WDDIS);
wdd->timeout = timeout;
@@ -186,13 +195,17 @@ static int sama5d4_wdt_init(struct sama5d4_wdt *wdt)
* If the watchdog is already running, we can safely update it.
* Else, we have to disable it properly.
*/
- if (wdt_enabled) {
+ reg = wdt_read(wdt, AT91_WDT_MR);
+ if (wdt_enabled(reg)) {
+ if (!wdt_enabled(wdt->mr))
+ wdt_write_nosleep(wdt, AT91_WDT_MR,
+ wdt->mr & ~AT91_WDT_WDDIS);
wdt_write_nosleep(wdt, AT91_WDT_MR, wdt->mr);
- } else {
- reg = wdt_read(wdt, AT91_WDT_MR);
- if (!(reg & AT91_WDT_WDDIS))
+ } else if (wdt_enabled(wdt->mr)) {
+ if (wdt_different_counters(reg, wdt->mr))
wdt_write_nosleep(wdt, AT91_WDT_MR,
- reg | AT91_WDT_WDDIS);
+ reg & ~AT91_WDT_WDDIS);
+ wdt_write_nosleep(wdt, AT91_WDT_MR, wdt->mr);
}
return 0;
}
--
2.17.1
[PATCH 0/2] Fixes for the SAMA5D2/SAMA5D4 watchdog
A previous change of the sama5d4_wdt driver broke the device probing with the device tree configuration described in existing DTS files, when no value is set for the "timeout-sec" property. Moreover, specifying any other value than 16 seconds for "timeout-sec" leads to a watchdog reset immediately when opening the watchdog device. Fix this by respecting hardware constraints added in recent versions of the SAMA5D2 and SAMA5D4 datasheet. Romain Izard (2): watchdog: sama5d4: fix timeout-sec usage watchdog: sama5d4: write the mode register in two steps drivers/watchdog/sama5d4_wdt.c | 33 + 1 file changed, 21 insertions(+), 12 deletions(-) -- 2.17.1
[PATCH 1/2] watchdog: sama5d4: fix timeout-sec usage
When using watchdog_init_timeout to update the default timeout value,
an error means that there is no "timeout-sec" in the relevant device
tree node.
This should not prevent binding of the driver to the device.
Fixes: 976932e40036 ("watchdog: sama5d4: make use of timeout-secs provided in
devicetree")
Signed-off-by: Romain Izard
---
drivers/watchdog/sama5d4_wdt.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 255169916dbb..1e93c1b0e3cf 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -247,11 +247,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
}
}
- ret = watchdog_init_timeout(wdd, wdt_timeout, >dev);
- if (ret) {
- dev_err(>dev, "unable to set timeout value\n");
- return ret;
- }
+ watchdog_init_timeout(wdd, wdt_timeout, >dev);
timeout = WDT_SEC2TICKS(wdd->timeout);
--
2.17.1
[PATCH 0/2] Fixes for the SAMA5D2/SAMA5D4 watchdog
A previous change of the sama5d4_wdt driver broke the device probing with the device tree configuration described in existing DTS files, when no value is set for the "timeout-sec" property. Moreover, specifying any other value than 16 seconds for "timeout-sec" leads to a watchdog reset immediately when opening the watchdog device. Fix this by respecting hardware constraints added in recent versions of the SAMA5D2 and SAMA5D4 datasheet. Romain Izard (2): watchdog: sama5d4: fix timeout-sec usage watchdog: sama5d4: write the mode register in two steps drivers/watchdog/sama5d4_wdt.c | 33 + 1 file changed, 21 insertions(+), 12 deletions(-) -- 2.17.1
[PATCH 1/2] watchdog: sama5d4: fix timeout-sec usage
When using watchdog_init_timeout to update the default timeout value,
an error means that there is no "timeout-sec" in the relevant device
tree node.
This should not prevent binding of the driver to the device.
Fixes: 976932e40036 ("watchdog: sama5d4: make use of timeout-secs provided in
devicetree")
Signed-off-by: Romain Izard
---
drivers/watchdog/sama5d4_wdt.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/watchdog/sama5d4_wdt.c b/drivers/watchdog/sama5d4_wdt.c
index 255169916dbb..1e93c1b0e3cf 100644
--- a/drivers/watchdog/sama5d4_wdt.c
+++ b/drivers/watchdog/sama5d4_wdt.c
@@ -247,11 +247,7 @@ static int sama5d4_wdt_probe(struct platform_device *pdev)
}
}
- ret = watchdog_init_timeout(wdd, wdt_timeout, >dev);
- if (ret) {
- dev_err(>dev, "unable to set timeout value\n");
- return ret;
- }
+ watchdog_init_timeout(wdd, wdt_timeout, >dev);
timeout = WDT_SEC2TICKS(wdd->timeout);
--
2.17.1
[PATCH v2 2/3] usb: gadget: udc: atmel: Remove obsolete include
The include defines the private platform_data structure used with AVR
platforms. It has no user since 7c55984e191f. Remove it.
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 -
include/linux/usb/atmel_usba_udc.h | 24
2 files changed, 25 deletions(-)
delete mode 100644 include/linux/usb/atmel_usba_udc.h
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 0fe3d0feb8f7..b9623e228609 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/usb/atmel_usba_udc.h
b/include/linux/usb/atmel_usba_udc.h
deleted file mode 100644
index 9bb00df3b53f..
--- a/include/linux/usb/atmel_usba_udc.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Platform data definitions for Atmel USBA gadget driver.
- */
-#ifndef __LINUX_USB_USBA_H
-#define __LINUX_USB_USBA_H
-
-struct usba_ep_data {
- char*name;
- int index;
- int fifo_size;
- int nr_banks;
- int can_dma;
- int can_isoc;
-};
-
-struct usba_platform_data {
- int vbus_pin;
- int vbus_pin_inverted;
- int num_ep;
- struct usba_ep_data ep[0];
-};
-
-#endif /* __LINUX_USB_USBA_H */
--
2.14.1
[PATCH v2 2/3] usb: gadget: udc: atmel: Remove obsolete include
The include defines the private platform_data structure used with AVR
platforms. It has no user since 7c55984e191f. Remove it.
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 -
include/linux/usb/atmel_usba_udc.h | 24
2 files changed, 25 deletions(-)
delete mode 100644 include/linux/usb/atmel_usba_udc.h
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 0fe3d0feb8f7..b9623e228609 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/usb/atmel_usba_udc.h
b/include/linux/usb/atmel_usba_udc.h
deleted file mode 100644
index 9bb00df3b53f..
--- a/include/linux/usb/atmel_usba_udc.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Platform data definitions for Atmel USBA gadget driver.
- */
-#ifndef __LINUX_USB_USBA_H
-#define __LINUX_USB_USBA_H
-
-struct usba_ep_data {
- char*name;
- int index;
- int fifo_size;
- int nr_banks;
- int can_dma;
- int can_isoc;
-};
-
-struct usba_platform_data {
- int vbus_pin;
- int vbus_pin_inverted;
- int num_ep;
- struct usba_ep_data ep[0];
-};
-
-#endif /* __LINUX_USB_USBA_H */
--
2.14.1
[PATCH v2 1/3] usb: gadget: udc: atmel: GPIO inversion is handled by gpiod
When converting to GPIO descriptors, gpiod_get_value automatically
handles the line inversion flags from the device tree.
Do not invert the line twice.
Fixes: 3df034081021 ("usb: gadget: udc: atmel: convert to use GPIO descriptors")
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 +--
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 27c16399c7e8..0fe3d0feb8f7 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -417,7 +417,7 @@ static inline void usba_int_enb_set(struct usba_udc *udc,
u32 val)
static int vbus_is_present(struct usba_udc *udc)
{
if (udc->vbus_pin)
- return gpiod_get_value(udc->vbus_pin) ^ udc->vbus_pin_inverted;
+ return gpiod_get_value(udc->vbus_pin);
/* No Vbus detection: Assume always present */
return 1;
@@ -2076,7 +2076,6 @@ static struct usba_ep * atmel_udc_of_init(struct
platform_device *pdev,
udc->vbus_pin = devm_gpiod_get_optional(>dev, "atmel,vbus",
GPIOD_IN);
- udc->vbus_pin_inverted = gpiod_is_active_low(udc->vbus_pin);
if (fifo_mode == 0) {
pp = NULL;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 969ce8f3c3e2..d7eb7cf4fd5c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -326,7 +326,6 @@ struct usba_udc {
const struct usba_udc_errata *errata;
int irq;
struct gpio_desc *vbus_pin;
- int vbus_pin_inverted;
int num_ep;
int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
--
2.14.1
[PATCH v2 1/3] usb: gadget: udc: atmel: GPIO inversion is handled by gpiod
When converting to GPIO descriptors, gpiod_get_value automatically
handles the line inversion flags from the device tree.
Do not invert the line twice.
Fixes: 3df034081021 ("usb: gadget: udc: atmel: convert to use GPIO descriptors")
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 +--
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 27c16399c7e8..0fe3d0feb8f7 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -417,7 +417,7 @@ static inline void usba_int_enb_set(struct usba_udc *udc,
u32 val)
static int vbus_is_present(struct usba_udc *udc)
{
if (udc->vbus_pin)
- return gpiod_get_value(udc->vbus_pin) ^ udc->vbus_pin_inverted;
+ return gpiod_get_value(udc->vbus_pin);
/* No Vbus detection: Assume always present */
return 1;
@@ -2076,7 +2076,6 @@ static struct usba_ep * atmel_udc_of_init(struct
platform_device *pdev,
udc->vbus_pin = devm_gpiod_get_optional(>dev, "atmel,vbus",
GPIOD_IN);
- udc->vbus_pin_inverted = gpiod_is_active_low(udc->vbus_pin);
if (fifo_mode == 0) {
pp = NULL;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 969ce8f3c3e2..d7eb7cf4fd5c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -326,7 +326,6 @@ struct usba_udc {
const struct usba_udc_errata *errata;
int irq;
struct gpio_desc *vbus_pin;
- int vbus_pin_inverted;
int num_ep;
int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
--
2.14.1
[PATCH v2 3/3] usb: gadget: udc: atmel: Fix indenting
Fix the fallout of the conversion to GPIO descriptors in 3df034081021.
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index b9623e228609..2f586f2bda7e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2277,15 +2277,15 @@ static int usba_udc_probe(struct platform_device *pdev)
if (udc->vbus_pin) {
irq_set_status_flags(gpiod_to_irq(udc->vbus_pin), IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(>dev,
- gpiod_to_irq(udc->vbus_pin), NULL,
- usba_vbus_irq_thread,
USBA_VBUS_IRQFLAGS,
- "atmel_usba_udc", udc);
- if (ret) {
- udc->vbus_pin = NULL;
- dev_warn(>pdev->dev,
-"failed to request vbus irq; "
-"assuming always on\n");
- }
+ gpiod_to_irq(udc->vbus_pin), NULL,
+ usba_vbus_irq_thread, USBA_VBUS_IRQFLAGS,
+ "atmel_usba_udc", udc);
+ if (ret) {
+ udc->vbus_pin = NULL;
+ dev_warn(>pdev->dev,
+"failed to request vbus irq; "
+"assuming always on\n");
+ }
}
ret = usb_add_gadget_udc(>dev, >gadget);
--
2.14.1
[PATCH v2 3/3] usb: gadget: udc: atmel: Fix indenting
Fix the fallout of the conversion to GPIO descriptors in 3df034081021.
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index b9623e228609..2f586f2bda7e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2277,15 +2277,15 @@ static int usba_udc_probe(struct platform_device *pdev)
if (udc->vbus_pin) {
irq_set_status_flags(gpiod_to_irq(udc->vbus_pin), IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(>dev,
- gpiod_to_irq(udc->vbus_pin), NULL,
- usba_vbus_irq_thread,
USBA_VBUS_IRQFLAGS,
- "atmel_usba_udc", udc);
- if (ret) {
- udc->vbus_pin = NULL;
- dev_warn(>pdev->dev,
-"failed to request vbus irq; "
-"assuming always on\n");
- }
+ gpiod_to_irq(udc->vbus_pin), NULL,
+ usba_vbus_irq_thread, USBA_VBUS_IRQFLAGS,
+ "atmel_usba_udc", udc);
+ if (ret) {
+ udc->vbus_pin = NULL;
+ dev_warn(>pdev->dev,
+"failed to request vbus irq; "
+"assuming always on\n");
+ }
}
ret = usb_add_gadget_udc(>dev, >gadget);
--
2.14.1
[PATCH v2 0/3] Fix an Atmel USBA UDC issue introducted in 4.17-rc1
The use of GPIO descriptors takes care of inversion flags declared in the device tree. The conversion of the Atmel USBA UDC driver introduced in 4.17-rc1 missed it, and as a result the inversion will not work. In addition, cleanup the code to remove an include left behind after the suppression of the support of platform_data to declare USBA controllers. These changes have not been tested on any hardware, as I do not have a board that needs to use inverted GPIOs. -- Changes in v2: - Use the correct format for the "Fixes:" tag - Collect "Acked-by:" tags Romain Izard (3): usb: gadget: udc: atmel: GPIO inversion is handled by gpiod usb: gadget: udc: atmel: Remove obsolete include usb: gadget: udc: atmel: Fix indenting drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++ drivers/usb/gadget/udc/atmel_usba_udc.h | 1 - include/linux/usb/atmel_usba_udc.h | 24 3 files changed, 10 insertions(+), 37 deletions(-) delete mode 100644 include/linux/usb/atmel_usba_udc.h -- 2.14.1
[PATCH v2 0/3] Fix an Atmel USBA UDC issue introducted in 4.17-rc1
The use of GPIO descriptors takes care of inversion flags declared in the device tree. The conversion of the Atmel USBA UDC driver introduced in 4.17-rc1 missed it, and as a result the inversion will not work. In addition, cleanup the code to remove an include left behind after the suppression of the support of platform_data to declare USBA controllers. These changes have not been tested on any hardware, as I do not have a board that needs to use inverted GPIOs. -- Changes in v2: - Use the correct format for the "Fixes:" tag - Collect "Acked-by:" tags Romain Izard (3): usb: gadget: udc: atmel: GPIO inversion is handled by gpiod usb: gadget: udc: atmel: Remove obsolete include usb: gadget: udc: atmel: Fix indenting drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++ drivers/usb/gadget/udc/atmel_usba_udc.h | 1 - include/linux/usb/atmel_usba_udc.h | 24 3 files changed, 10 insertions(+), 37 deletions(-) delete mode 100644 include/linux/usb/atmel_usba_udc.h -- 2.14.1
[PATCH 2/3] usb: gadget: udc: atmel: Remove obsolete include
The include defines the private platform_data structure used with AVR
platforms. It has no user since 7c55984e191f. Remove it.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 -
include/linux/usb/atmel_usba_udc.h | 24
2 files changed, 25 deletions(-)
delete mode 100644 include/linux/usb/atmel_usba_udc.h
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 0fe3d0feb8f7..b9623e228609 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/usb/atmel_usba_udc.h
b/include/linux/usb/atmel_usba_udc.h
deleted file mode 100644
index 9bb00df3b53f..
--- a/include/linux/usb/atmel_usba_udc.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Platform data definitions for Atmel USBA gadget driver.
- */
-#ifndef __LINUX_USB_USBA_H
-#define __LINUX_USB_USBA_H
-
-struct usba_ep_data {
- char*name;
- int index;
- int fifo_size;
- int nr_banks;
- int can_dma;
- int can_isoc;
-};
-
-struct usba_platform_data {
- int vbus_pin;
- int vbus_pin_inverted;
- int num_ep;
- struct usba_ep_data ep[0];
-};
-
-#endif /* __LINUX_USB_USBA_H */
--
2.14.1
[PATCH 2/3] usb: gadget: udc: atmel: Remove obsolete include
The include defines the private platform_data structure used with AVR
platforms. It has no user since 7c55984e191f. Remove it.
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 1 -
include/linux/usb/atmel_usba_udc.h | 24
2 files changed, 25 deletions(-)
delete mode 100644 include/linux/usb/atmel_usba_udc.h
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 0fe3d0feb8f7..b9623e228609 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -20,7 +20,6 @@
#include
#include
#include
-#include
#include
#include
#include
diff --git a/include/linux/usb/atmel_usba_udc.h
b/include/linux/usb/atmel_usba_udc.h
deleted file mode 100644
index 9bb00df3b53f..
--- a/include/linux/usb/atmel_usba_udc.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Platform data definitions for Atmel USBA gadget driver.
- */
-#ifndef __LINUX_USB_USBA_H
-#define __LINUX_USB_USBA_H
-
-struct usba_ep_data {
- char*name;
- int index;
- int fifo_size;
- int nr_banks;
- int can_dma;
- int can_isoc;
-};
-
-struct usba_platform_data {
- int vbus_pin;
- int vbus_pin_inverted;
- int num_ep;
- struct usba_ep_data ep[0];
-};
-
-#endif /* __LINUX_USB_USBA_H */
--
2.14.1
[PATCH 3/3] usb: gadget: udc: atmel: Fix indenting
Fix the fallout of the conversion to GPIO descriptors in 3df034081021.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index b9623e228609..2f586f2bda7e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2277,15 +2277,15 @@ static int usba_udc_probe(struct platform_device *pdev)
if (udc->vbus_pin) {
irq_set_status_flags(gpiod_to_irq(udc->vbus_pin), IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(>dev,
- gpiod_to_irq(udc->vbus_pin), NULL,
- usba_vbus_irq_thread,
USBA_VBUS_IRQFLAGS,
- "atmel_usba_udc", udc);
- if (ret) {
- udc->vbus_pin = NULL;
- dev_warn(>pdev->dev,
-"failed to request vbus irq; "
-"assuming always on\n");
- }
+ gpiod_to_irq(udc->vbus_pin), NULL,
+ usba_vbus_irq_thread, USBA_VBUS_IRQFLAGS,
+ "atmel_usba_udc", udc);
+ if (ret) {
+ udc->vbus_pin = NULL;
+ dev_warn(>pdev->dev,
+"failed to request vbus irq; "
+"assuming always on\n");
+ }
}
ret = usb_add_gadget_udc(>dev, >gadget);
--
2.14.1
[PATCH 3/3] usb: gadget: udc: atmel: Fix indenting
Fix the fallout of the conversion to GPIO descriptors in 3df034081021.
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index b9623e228609..2f586f2bda7e 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2277,15 +2277,15 @@ static int usba_udc_probe(struct platform_device *pdev)
if (udc->vbus_pin) {
irq_set_status_flags(gpiod_to_irq(udc->vbus_pin), IRQ_NOAUTOEN);
ret = devm_request_threaded_irq(>dev,
- gpiod_to_irq(udc->vbus_pin), NULL,
- usba_vbus_irq_thread,
USBA_VBUS_IRQFLAGS,
- "atmel_usba_udc", udc);
- if (ret) {
- udc->vbus_pin = NULL;
- dev_warn(>pdev->dev,
-"failed to request vbus irq; "
-"assuming always on\n");
- }
+ gpiod_to_irq(udc->vbus_pin), NULL,
+ usba_vbus_irq_thread, USBA_VBUS_IRQFLAGS,
+ "atmel_usba_udc", udc);
+ if (ret) {
+ udc->vbus_pin = NULL;
+ dev_warn(>pdev->dev,
+"failed to request vbus irq; "
+"assuming always on\n");
+ }
}
ret = usb_add_gadget_udc(>dev, >gadget);
--
2.14.1
[PATCH 1/3] usb: gadget: udc: atmel: GPIO inversion is handled by gpiod
When converting to GPIO descriptors, gpiod_get_value automatically
handles the line inversion flags from the device tree.
Do not invert the line twice.
Fixes: 3df034081021fa4b6967ce3364bc7d867ec1c870
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 +--
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 27c16399c7e8..0fe3d0feb8f7 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -417,7 +417,7 @@ static inline void usba_int_enb_set(struct usba_udc *udc,
u32 val)
static int vbus_is_present(struct usba_udc *udc)
{
if (udc->vbus_pin)
- return gpiod_get_value(udc->vbus_pin) ^ udc->vbus_pin_inverted;
+ return gpiod_get_value(udc->vbus_pin);
/* No Vbus detection: Assume always present */
return 1;
@@ -2076,7 +2076,6 @@ static struct usba_ep * atmel_udc_of_init(struct
platform_device *pdev,
udc->vbus_pin = devm_gpiod_get_optional(>dev, "atmel,vbus",
GPIOD_IN);
- udc->vbus_pin_inverted = gpiod_is_active_low(udc->vbus_pin);
if (fifo_mode == 0) {
pp = NULL;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 969ce8f3c3e2..d7eb7cf4fd5c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -326,7 +326,6 @@ struct usba_udc {
const struct usba_udc_errata *errata;
int irq;
struct gpio_desc *vbus_pin;
- int vbus_pin_inverted;
int num_ep;
int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
--
2.14.1
[PATCH 1/3] usb: gadget: udc: atmel: GPIO inversion is handled by gpiod
When converting to GPIO descriptors, gpiod_get_value automatically
handles the line inversion flags from the device tree.
Do not invert the line twice.
Fixes: 3df034081021fa4b6967ce3364bc7d867ec1c870
Signed-off-by: Romain Izard
---
drivers/usb/gadget/udc/atmel_usba_udc.c | 3 +--
drivers/usb/gadget/udc/atmel_usba_udc.h | 1 -
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 27c16399c7e8..0fe3d0feb8f7 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -417,7 +417,7 @@ static inline void usba_int_enb_set(struct usba_udc *udc,
u32 val)
static int vbus_is_present(struct usba_udc *udc)
{
if (udc->vbus_pin)
- return gpiod_get_value(udc->vbus_pin) ^ udc->vbus_pin_inverted;
+ return gpiod_get_value(udc->vbus_pin);
/* No Vbus detection: Assume always present */
return 1;
@@ -2076,7 +2076,6 @@ static struct usba_ep * atmel_udc_of_init(struct
platform_device *pdev,
udc->vbus_pin = devm_gpiod_get_optional(>dev, "atmel,vbus",
GPIOD_IN);
- udc->vbus_pin_inverted = gpiod_is_active_low(udc->vbus_pin);
if (fifo_mode == 0) {
pp = NULL;
diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.h
b/drivers/usb/gadget/udc/atmel_usba_udc.h
index 969ce8f3c3e2..d7eb7cf4fd5c 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.h
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.h
@@ -326,7 +326,6 @@ struct usba_udc {
const struct usba_udc_errata *errata;
int irq;
struct gpio_desc *vbus_pin;
- int vbus_pin_inverted;
int num_ep;
int configured_ep;
struct usba_fifo_cfg *fifo_cfg;
--
2.14.1
[PATCH 0/3] Fix an Atmel USBA UDC issue introducted in 4.17-rc1
The use of GPIO descriptors takes care of inversion flags declared in the device tree. The conversion of the Atmel USBA UDC driver introduced in 4.17-rc1 missed it, and as a result the inversion will not work. In addition, cleanup the code to remove an include left behind after the suppression of the support of platform_data to declare USBA controllers. These changes have not been tested on any hardware, as I do not have a board that needs to use inverted GPIOs. Romain Izard (3): usb: gadget: udc: atmel: GPIO inversion is handled by gpiod usb: gadget: udc: atmel: Remove obsolete include usb: gadget: udc: atmel: Fix indenting drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++ drivers/usb/gadget/udc/atmel_usba_udc.h | 1 - include/linux/usb/atmel_usba_udc.h | 24 3 files changed, 10 insertions(+), 37 deletions(-) delete mode 100644 include/linux/usb/atmel_usba_udc.h -- 2.14.1
[PATCH 0/3] Fix an Atmel USBA UDC issue introducted in 4.17-rc1
The use of GPIO descriptors takes care of inversion flags declared in the device tree. The conversion of the Atmel USBA UDC driver introduced in 4.17-rc1 missed it, and as a result the inversion will not work. In addition, cleanup the code to remove an include left behind after the suppression of the support of platform_data to declare USBA controllers. These changes have not been tested on any hardware, as I do not have a board that needs to use inverted GPIOs. Romain Izard (3): usb: gadget: udc: atmel: GPIO inversion is handled by gpiod usb: gadget: udc: atmel: Remove obsolete include usb: gadget: udc: atmel: Fix indenting drivers/usb/gadget/udc/atmel_usba_udc.c | 22 ++ drivers/usb/gadget/udc/atmel_usba_udc.h | 1 - include/linux/usb/atmel_usba_udc.h | 24 3 files changed, 10 insertions(+), 37 deletions(-) delete mode 100644 include/linux/usb/atmel_usba_udc.h -- 2.14.1
[RFC PATCH] cdc-acm: do not drop data from fast devices
There are some devices using their USB CDC-ACM interfaces as a debug port
that are able to send data at a very high speed, but with the current
driver implementation it is not possible to receive it when using a
relatively slow embedded system without dropping an important part of the
data.
The existing driver uses the throttling mechanism of the TTY line
discipline to regulate the speed of the data transmitted from the port to
the upper layers. This throttling prevents URBs from being resubmitted,
slowing down the transfer on the USB line. But the existing code does not
work correctly when the internal bufferring gets filled.
The TTY buffer is 4096 bytes large, throttling when there are only 128
free bytes left, and unthrottling when there are only 128 bytes available.
But the TTY buffer is filled from an intermediate flip buffer that
contains up to 64 KiB of data, and each time unthrottle() is called 16
URBs are scheduled by the driver, sending up to 16 KiB to the flip buffer.
As the result of tty_insert_flip_string() is not checked in the URB
reception callback, data can be lost when the flip buffer is filled faster
than the TTY is emptied.
Moreover, as the URB callbacks are called from a tasklet context, whereas
throttling is called from the system workqueue, it is possible for the
throttling to be delayed by high traffic in the tasklet. As a result,
completed URBs can be resubmitted even if the flip buffer is full, and we
request more data from the device only to drop it immediately.
To prevent this problem, the URBs whose data did not reach the flip buffer
are placed in a waiting list, which is only processed when the serial port
is unthrottled.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
--
This is working when using the normal line discipline on ttyACM. But
there is a big hole in this: other line disciplines do not use throttling
and thus unthrottle is never called. The URBs will never get resubmitted,
and the port is dead. Unfortunately, there is no notification when the
flip buffer is ready to receive new data, so the only alternative would
be to schedule a timer polling the flip buffer. But with an additional
asynchronous process in the mix, the code starts to be very brittle.
I believe this issue is not limited to ttyACM. As the TTY layer is not
performance-oriented, it can be easy to overwhelm devices with a low
available processing power. In my case, a modem sending a sustained 2 MB/s
on a debug port (exported as a CDC-ACM port) was enough to trigger the
issue. The code handling the CDC-ACM class and the generic USB serial port
is very similar when it comes to URB handling, so all drivers that rely on
that code have the same issue.
But in general, it seems to me that there is no code in the kernel that
checks the return value of tty_insert_flip_string(). This means that we
are working with the assumption that the kernel will consume the data
faster than the source can send it, or that the upper layer will be
able or willing to throttle it fast enough to avoid losing data.
---
drivers/usb/class/cdc-acm.c | 80 -
drivers/usb/class/cdc-acm.h | 4 +++
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 7b366a6c0b49..833fa0a43ddd 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -150,12 +150,18 @@ static inline int acm_set_control(struct acm *acm, int
control)
static void acm_kill_urbs(struct acm *acm)
{
int i;
+ struct acm_rb *rb, *t;
usb_kill_urb(acm->ctrlurb);
for (i = 0; i < ACM_NW; i++)
usb_kill_urb(acm->wb[i].urb);
for (i = 0; i < acm->rx_buflimit; i++)
usb_kill_urb(acm->read_urbs[i]);
+ list_for_each_entry_safe(rb, t, >wait_list, node) {
+ set_bit(rb->index, >read_urbs_free);
+ rb->offset = 0;
+ list_del_init(>node);
+ }
}
/*
@@ -454,14 +460,27 @@ static int acm_submit_read_urbs(struct acm *acm, gfp_t
mem_flags)
return 0;
}
-static void acm_process_read_urb(struct acm *acm, struct urb *urb)
+static int acm_process_read_urb(struct acm *acm, struct urb *urb)
{
+ int flipped, remainder;
+ struct acm_rb *rb = urb->context;
if (!urb->actual_length)
- return;
+ return 0;
+ flipped = tty_insert_flip_string(>port,
+ urb->transfer_buffer + rb->offset,
+ urb->actual_length - rb->offset);
+ rb->offset += flipped;
+ remainder = urb->actual_length - rb->offset;
+ if (remainder != 0)
+ dev_dbg(>data->dev,
+ "remaining data: usb %d len %d offset %d flipped %d\n",
+ rb->index, urb->actual_length, rb->offset, flip
[RFC PATCH] cdc-acm: do not drop data from fast devices
There are some devices using their USB CDC-ACM interfaces as a debug port
that are able to send data at a very high speed, but with the current
driver implementation it is not possible to receive it when using a
relatively slow embedded system without dropping an important part of the
data.
The existing driver uses the throttling mechanism of the TTY line
discipline to regulate the speed of the data transmitted from the port to
the upper layers. This throttling prevents URBs from being resubmitted,
slowing down the transfer on the USB line. But the existing code does not
work correctly when the internal bufferring gets filled.
The TTY buffer is 4096 bytes large, throttling when there are only 128
free bytes left, and unthrottling when there are only 128 bytes available.
But the TTY buffer is filled from an intermediate flip buffer that
contains up to 64 KiB of data, and each time unthrottle() is called 16
URBs are scheduled by the driver, sending up to 16 KiB to the flip buffer.
As the result of tty_insert_flip_string() is not checked in the URB
reception callback, data can be lost when the flip buffer is filled faster
than the TTY is emptied.
Moreover, as the URB callbacks are called from a tasklet context, whereas
throttling is called from the system workqueue, it is possible for the
throttling to be delayed by high traffic in the tasklet. As a result,
completed URBs can be resubmitted even if the flip buffer is full, and we
request more data from the device only to drop it immediately.
To prevent this problem, the URBs whose data did not reach the flip buffer
are placed in a waiting list, which is only processed when the serial port
is unthrottled.
Signed-off-by: Romain Izard
--
This is working when using the normal line discipline on ttyACM. But
there is a big hole in this: other line disciplines do not use throttling
and thus unthrottle is never called. The URBs will never get resubmitted,
and the port is dead. Unfortunately, there is no notification when the
flip buffer is ready to receive new data, so the only alternative would
be to schedule a timer polling the flip buffer. But with an additional
asynchronous process in the mix, the code starts to be very brittle.
I believe this issue is not limited to ttyACM. As the TTY layer is not
performance-oriented, it can be easy to overwhelm devices with a low
available processing power. In my case, a modem sending a sustained 2 MB/s
on a debug port (exported as a CDC-ACM port) was enough to trigger the
issue. The code handling the CDC-ACM class and the generic USB serial port
is very similar when it comes to URB handling, so all drivers that rely on
that code have the same issue.
But in general, it seems to me that there is no code in the kernel that
checks the return value of tty_insert_flip_string(). This means that we
are working with the assumption that the kernel will consume the data
faster than the source can send it, or that the upper layer will be
able or willing to throttle it fast enough to avoid losing data.
---
drivers/usb/class/cdc-acm.c | 80 -
drivers/usb/class/cdc-acm.h | 4 +++
2 files changed, 75 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 7b366a6c0b49..833fa0a43ddd 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -150,12 +150,18 @@ static inline int acm_set_control(struct acm *acm, int
control)
static void acm_kill_urbs(struct acm *acm)
{
int i;
+ struct acm_rb *rb, *t;
usb_kill_urb(acm->ctrlurb);
for (i = 0; i < ACM_NW; i++)
usb_kill_urb(acm->wb[i].urb);
for (i = 0; i < acm->rx_buflimit; i++)
usb_kill_urb(acm->read_urbs[i]);
+ list_for_each_entry_safe(rb, t, >wait_list, node) {
+ set_bit(rb->index, >read_urbs_free);
+ rb->offset = 0;
+ list_del_init(>node);
+ }
}
/*
@@ -454,14 +460,27 @@ static int acm_submit_read_urbs(struct acm *acm, gfp_t
mem_flags)
return 0;
}
-static void acm_process_read_urb(struct acm *acm, struct urb *urb)
+static int acm_process_read_urb(struct acm *acm, struct urb *urb)
{
+ int flipped, remainder;
+ struct acm_rb *rb = urb->context;
if (!urb->actual_length)
- return;
+ return 0;
+ flipped = tty_insert_flip_string(>port,
+ urb->transfer_buffer + rb->offset,
+ urb->actual_length - rb->offset);
+ rb->offset += flipped;
+ remainder = urb->actual_length - rb->offset;
+ if (remainder != 0)
+ dev_dbg(>data->dev,
+ "remaining data: usb %d len %d offset %d flipped %d\n",
+ rb->index, urb->actual_length, rb->offset, flipped);
+ else
+
[PATCH RESEND] mtd: ubi: block: Fix error for write access
When opening a device with write access, ubiblock_open returns an error
code. Currently, this error code is -EPERM, but this is not the right
value.
The open function for other block devices returns -EROFS when opening
read-only devices with FMODE_WRITE set. When used with dm-verity, the
veritysetup userspace tool is expecting EROFS, and refuses to use the
ubiblock device.
Use -EROFS for ubiblock as well. As a result, veritysetup accepts the
ubiblock device as valid.
Fixes: 9d54c8a33eec (UBI: R/O block driver on top of UBI volumes)
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/mtd/ubi/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index b210fdb31c98..4533423cf2aa 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -242,7 +242,7 @@ static int ubiblock_open(struct block_device *bdev, fmode_t
mode)
* in any case.
*/
if (mode & FMODE_WRITE) {
- ret = -EPERM;
+ ret = -EROFS;
goto out_unlock;
}
--
2.14.1
[PATCH RESEND] mtd: ubi: block: Fix error for write access
When opening a device with write access, ubiblock_open returns an error
code. Currently, this error code is -EPERM, but this is not the right
value.
The open function for other block devices returns -EROFS when opening
read-only devices with FMODE_WRITE set. When used with dm-verity, the
veritysetup userspace tool is expecting EROFS, and refuses to use the
ubiblock device.
Use -EROFS for ubiblock as well. As a result, veritysetup accepts the
ubiblock device as valid.
Fixes: 9d54c8a33eec (UBI: R/O block driver on top of UBI volumes)
Signed-off-by: Romain Izard
---
drivers/mtd/ubi/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index b210fdb31c98..4533423cf2aa 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -242,7 +242,7 @@ static int ubiblock_open(struct block_device *bdev, fmode_t
mode)
* in any case.
*/
if (mode & FMODE_WRITE) {
- ret = -EPERM;
+ ret = -EROFS;
goto out_unlock;
}
--
2.14.1
[PATCH v1] mtd: ubi: block: Fix error for write access
When opening a device with write access, ubiblock_open returns an error
code. Currently, this error code is -EPERM, but this is not the right
value.
The open function for other block devices returns -EROFS when opening
read-only devices with FMODE_WRITE set. When used with dm-verity, the
veritysetup userspace tool is expecting EROFS, and refuses to use the
ubiblock device.
Use -EROFS for ubiblock as well. As a result, veritysetup accepts the
ubiblock device as valid.
Fixes: 9d54c8a33eec (UBI: R/O block driver on top of UBI volumes)
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/mtd/ubi/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index b210fdb31c98..4533423cf2aa 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -242,7 +242,7 @@ static int ubiblock_open(struct block_device *bdev, fmode_t
mode)
* in any case.
*/
if (mode & FMODE_WRITE) {
- ret = -EPERM;
+ ret = -EROFS;
goto out_unlock;
}
--
2.14.1
[PATCH v1] mtd: ubi: block: Fix error for write access
When opening a device with write access, ubiblock_open returns an error
code. Currently, this error code is -EPERM, but this is not the right
value.
The open function for other block devices returns -EROFS when opening
read-only devices with FMODE_WRITE set. When used with dm-verity, the
veritysetup userspace tool is expecting EROFS, and refuses to use the
ubiblock device.
Use -EROFS for ubiblock as well. As a result, veritysetup accepts the
ubiblock device as valid.
Fixes: 9d54c8a33eec (UBI: R/O block driver on top of UBI volumes)
Signed-off-by: Romain Izard
---
drivers/mtd/ubi/block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index b210fdb31c98..4533423cf2aa 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -242,7 +242,7 @@ static int ubiblock_open(struct block_device *bdev, fmode_t
mode)
* in any case.
*/
if (mode & FMODE_WRITE) {
- ret = -EPERM;
+ ret = -EROFS;
goto out_unlock;
}
--
2.14.1
"perf record" leads to a watchdog reset on a SAMA5D2 SoC
Hello, While trying to use perf on a device based on a SAMA5D2 SoC, I encountered a systematic watchdog reboot. Simply calling "perf record /bin/true" is enough to trigger this condition. I have reproduced the issue on a SAMA5D2 Xplained demo board, with a 4.14.11 kernel, leading to "rcu_preempt self-detected stall on CPU" kernel messages once the watchdog was disabled. But this was depending on the kernel configuration - the sama5_defconfig configuration would not show the issue, but adding the following options to this defconfig was be enough: +CONFIG_PERF_EVENTS=y +CONFIG_PREEMPT=y +CONFIG_FUNCTION_TRACER=y +# CONFIG_DYNAMIC_FTRACE is not set There are complex interactions between the options, and in the case of the original device DYNAMIC_FTRACE was set. But when trying to reduce the number of changed flags between the default and a configuration reproducing the issue, I ended up in a situation where enabling or disabling DYNAMIC_FTRACE is the difference that triggered the problem. Here is an example of the error (repeated every 30s) in the kernel log: 8<-- INFO: rcu_preempt self-detected stall on CPU 0-...: (1413 ticks this GP) idle=7fe/142/0 softirq=1011/1011 fqs=0 (t=2100 jiffies g=86 c=85 q=63) rcu_preempt kthread starved for 2100 jiffies! g86 c85 f0x0 RCU_GP_WAIT_FQS(3) ->state=0x402 ->cpu=0 rcu_preempt I0 8 2 0x [] (__schedule) from [] (schedule+0x58/0xb8) [] (schedule) from [] (schedule_timeout+0x1d8/0x3fc) [] (schedule_timeout) from [] (rcu_gp_kthread+0x628/0xab4) [] (rcu_gp_kthread) from [] (kthread+0x134/0x164) [] (kthread) from [] (ret_from_fork+0x14/0x2c) Task dump for CPU 0: perfR running task0 842836 0x0002 [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (sched_show_task.part.22+0xc4/0xe0) [] (sched_show_task.part.22) from [] (dump_cpu_task+0x40/0x44) [] (dump_cpu_task) from [] (rcu_dump_cpu_stacks+0x8c/0xcc) [] (rcu_dump_cpu_stacks) from [] (rcu_check_callbacks+0x628/0xa5c) [] (rcu_check_callbacks) from [] (update_process_times+0x40/0x6c) [] (update_process_times) from [] (tick_sched_handle+0x64/0x68) [] (tick_sched_handle) from [] (tick_sched_timer+0x58/0xb4) [] (tick_sched_timer) from [] (__hrtimer_run_queues.constprop.5+0x174/0x330) [] (__hrtimer_run_queues.constprop.5) from [] (hrtimer_interrupt+0xa4/0x260) [] (hrtimer_interrupt) from [] (ch2_irq+0x38/0x40) [] (ch2_irq) from [] (__handle_irq_event_percpu+0x6c/0x258) [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x2c/0x68) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x5c/0x94) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xbc/0x198) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x34/0x44) [] (generic_handle_irq) from [] (__handle_domain_irq+0x80/0xec) [] (__handle_domain_irq) from [] (aic5_handle+0xd0/0xf8) [] (aic5_handle) from [] (__irq_svc+0x6c/0xa8) Exception stack(0xde63fd18 to 0xde63fd60) fd00: c0a077dc fd20: c0d86200 c0d86200 0202 de63fe20 df007600 fd40: c0d7c3cc de63fdbc de63fd58 de63fd68 c03d3cb4 c0101648 200b0113 [] (__irq_svc) from [] (__do_softirq+0xa0/0x350) [] (__do_softirq) from [] (irq_exit+0xfc/0x138) [] (irq_exit) from [] (__handle_domain_irq+0x84/0xec) [] (__handle_domain_irq) from [] (aic5_handle+0xd0/0xf8) [] (aic5_handle) from [] (__irq_svc+0x6c/0xa8) Exception stack(0xde63fe20 to 0xde63fe68) fe20: c01c265c 600b0013 c01c612c de57f600 de63e000 fe40: df284c00 de63fe8c de63fe08 de63fe70 c0145178 c0187214 fe60: 600b0013 [] (__irq_svc) from [] (smp_call_function_single+0x38/0x60) [] (smp_call_function_single) from [] (task_function_call+0x3c/0x54) [] (task_function_call) from [] (perf_install_in_context+0x5c/0x19c) [] (perf_install_in_context) from [] (SyS_perf_event_open+0xb88/0xdcc) [] (SyS_perf_event_open) from [] (ret_fast_syscall+0x0/0x54) 8<-- I'm not sure of how to interpret the issue. Is it because the CPU is too busy servicing the trace code during ticks to do anything else? Best regards, -- Romain Izard
"perf record" leads to a watchdog reset on a SAMA5D2 SoC
Hello, While trying to use perf on a device based on a SAMA5D2 SoC, I encountered a systematic watchdog reboot. Simply calling "perf record /bin/true" is enough to trigger this condition. I have reproduced the issue on a SAMA5D2 Xplained demo board, with a 4.14.11 kernel, leading to "rcu_preempt self-detected stall on CPU" kernel messages once the watchdog was disabled. But this was depending on the kernel configuration - the sama5_defconfig configuration would not show the issue, but adding the following options to this defconfig was be enough: +CONFIG_PERF_EVENTS=y +CONFIG_PREEMPT=y +CONFIG_FUNCTION_TRACER=y +# CONFIG_DYNAMIC_FTRACE is not set There are complex interactions between the options, and in the case of the original device DYNAMIC_FTRACE was set. But when trying to reduce the number of changed flags between the default and a configuration reproducing the issue, I ended up in a situation where enabling or disabling DYNAMIC_FTRACE is the difference that triggered the problem. Here is an example of the error (repeated every 30s) in the kernel log: 8<-- INFO: rcu_preempt self-detected stall on CPU 0-...: (1413 ticks this GP) idle=7fe/142/0 softirq=1011/1011 fqs=0 (t=2100 jiffies g=86 c=85 q=63) rcu_preempt kthread starved for 2100 jiffies! g86 c85 f0x0 RCU_GP_WAIT_FQS(3) ->state=0x402 ->cpu=0 rcu_preempt I0 8 2 0x [] (__schedule) from [] (schedule+0x58/0xb8) [] (schedule) from [] (schedule_timeout+0x1d8/0x3fc) [] (schedule_timeout) from [] (rcu_gp_kthread+0x628/0xab4) [] (rcu_gp_kthread) from [] (kthread+0x134/0x164) [] (kthread) from [] (ret_from_fork+0x14/0x2c) Task dump for CPU 0: perfR running task0 842836 0x0002 [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (sched_show_task.part.22+0xc4/0xe0) [] (sched_show_task.part.22) from [] (dump_cpu_task+0x40/0x44) [] (dump_cpu_task) from [] (rcu_dump_cpu_stacks+0x8c/0xcc) [] (rcu_dump_cpu_stacks) from [] (rcu_check_callbacks+0x628/0xa5c) [] (rcu_check_callbacks) from [] (update_process_times+0x40/0x6c) [] (update_process_times) from [] (tick_sched_handle+0x64/0x68) [] (tick_sched_handle) from [] (tick_sched_timer+0x58/0xb4) [] (tick_sched_timer) from [] (__hrtimer_run_queues.constprop.5+0x174/0x330) [] (__hrtimer_run_queues.constprop.5) from [] (hrtimer_interrupt+0xa4/0x260) [] (hrtimer_interrupt) from [] (ch2_irq+0x38/0x40) [] (ch2_irq) from [] (__handle_irq_event_percpu+0x6c/0x258) [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x2c/0x68) [] (handle_irq_event_percpu) from [] (handle_irq_event+0x5c/0x94) [] (handle_irq_event) from [] (handle_fasteoi_irq+0xbc/0x198) [] (handle_fasteoi_irq) from [] (generic_handle_irq+0x34/0x44) [] (generic_handle_irq) from [] (__handle_domain_irq+0x80/0xec) [] (__handle_domain_irq) from [] (aic5_handle+0xd0/0xf8) [] (aic5_handle) from [] (__irq_svc+0x6c/0xa8) Exception stack(0xde63fd18 to 0xde63fd60) fd00: c0a077dc fd20: c0d86200 c0d86200 0202 de63fe20 df007600 fd40: c0d7c3cc de63fdbc de63fd58 de63fd68 c03d3cb4 c0101648 200b0113 [] (__irq_svc) from [] (__do_softirq+0xa0/0x350) [] (__do_softirq) from [] (irq_exit+0xfc/0x138) [] (irq_exit) from [] (__handle_domain_irq+0x84/0xec) [] (__handle_domain_irq) from [] (aic5_handle+0xd0/0xf8) [] (aic5_handle) from [] (__irq_svc+0x6c/0xa8) Exception stack(0xde63fe20 to 0xde63fe68) fe20: c01c265c 600b0013 c01c612c de57f600 de63e000 fe40: df284c00 de63fe8c de63fe08 de63fe70 c0145178 c0187214 fe60: 600b0013 [] (__irq_svc) from [] (smp_call_function_single+0x38/0x60) [] (smp_call_function_single) from [] (task_function_call+0x3c/0x54) [] (task_function_call) from [] (perf_install_in_context+0x5c/0x19c) [] (perf_install_in_context) from [] (SyS_perf_event_open+0xb88/0xdcc) [] (SyS_perf_event_open) from [] (ret_fast_syscall+0x0/0x54) 8<-- I'm not sure of how to interpret the issue. Is it because the CPU is too busy servicing the trace code during ticks to do anything else? Best regards, -- Romain Izard
[tip:timers/core] clocksource/drivers/tcb_clksrc: Fix clock speed message
Commit-ID: 542f824607a6968ea443208ccfef3b7daf503559
Gitweb: https://git.kernel.org/tip/542f824607a6968ea443208ccfef3b7daf503559
Author: Romain Izard <romain.izard@gmail.com>
AuthorDate: Mon, 8 Jan 2018 14:28:43 +0100
Committer: Ingo Molnar <mi...@kernel.org>
CommitDate: Mon, 8 Jan 2018 17:57:23 +0100
clocksource/drivers/tcb_clksrc: Fix clock speed message
The clock speed displayed at boot in an information message was 500 kHz
too high compared to its real value. As the value is not used anywhere,
there is no functional impact.
Fix the rounding formula to display the correct value.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Signed-off-by: Daniel Lezcano <daniel.lezc...@linaro.org>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link:
http://lkml.kernel.org/r/1515418139-23276-4-git-send-email-daniel.lezc...@linaro.org
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
drivers/clocksource/tcb_clksrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 9de47d4..43f4d5c 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -384,7 +384,7 @@ static int __init tcb_clksrc_init(void)
printk(bootinfo, clksrc.name, CONFIG_ATMEL_TCB_CLKSRC_BLOCK,
divided_rate / 100,
- ((divided_rate + 50) % 100) / 1000);
+ ((divided_rate % 100) + 500) / 1000);
if (tc->tcb_config && tc->tcb_config->counter_width == 32) {
/* use apropriate function to read 32 bit counter */
[tip:timers/core] clocksource/drivers/tcb_clksrc: Fix clock speed message
Commit-ID: 542f824607a6968ea443208ccfef3b7daf503559
Gitweb: https://git.kernel.org/tip/542f824607a6968ea443208ccfef3b7daf503559
Author: Romain Izard
AuthorDate: Mon, 8 Jan 2018 14:28:43 +0100
Committer: Ingo Molnar
CommitDate: Mon, 8 Jan 2018 17:57:23 +0100
clocksource/drivers/tcb_clksrc: Fix clock speed message
The clock speed displayed at boot in an information message was 500 kHz
too high compared to its real value. As the value is not used anywhere,
there is no functional impact.
Fix the rounding formula to display the correct value.
Signed-off-by: Romain Izard
Signed-off-by: Daniel Lezcano
Acked-by: Nicolas Ferre
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Link:
http://lkml.kernel.org/r/1515418139-23276-4-git-send-email-daniel.lezc...@linaro.org
Signed-off-by: Ingo Molnar
---
drivers/clocksource/tcb_clksrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 9de47d4..43f4d5c 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -384,7 +384,7 @@ static int __init tcb_clksrc_init(void)
printk(bootinfo, clksrc.name, CONFIG_ATMEL_TCB_CLKSRC_BLOCK,
divided_rate / 100,
- ((divided_rate + 50) % 100) / 1000);
+ ((divided_rate % 100) + 500) / 1000);
if (tc->tcb_config && tc->tcb_config->counter_width == 32) {
/* use apropriate function to read 32 bit counter */
[PATCH v6] atmel_flexcom: Support resuming after a chip reset
The controller used by a flexcom module is configured at boot, and left
alone after this. In the suspend mode called "backup with self-refresh"
available on SAMA5D2, the chip will resume with most of its registers
reset. In this case, we need to restore the state of the flexcom driver
on resume.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
Changes in v6:
* Reword the patch title and description
* Rename the internal structure to ddata
drivers/mfd/atmel-flexcom.c | 63 ++---
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..f684a93a3340 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,43 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *ddata;
int err;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ ddata = devm_kzalloc(>dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ddata);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (ddata->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ ddata->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ ddata->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(ddata->base))
+ return PTR_ERR(ddata->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ ddata->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(ddata->clk))
+ return PTR_ERR(ddata->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(ddata->clk);
if (err)
return err;
@@ -76,9 +85,9 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ writel(FLEX_MR_OPMODE(ddata->opmode), ddata->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(ddata->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +98,34 @@ static const struct of_device_id atmel_flexcom_of_match[] =
{
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *ddata = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(ddata->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(ddata->opmode),
+ writel(val, ddata->base + FLEX_MR);
+
+ clk_disable_unprepare(ddata->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.14.1
[PATCH v6] atmel_flexcom: Support resuming after a chip reset
The controller used by a flexcom module is configured at boot, and left
alone after this. In the suspend mode called "backup with self-refresh"
available on SAMA5D2, the chip will resume with most of its registers
reset. In this case, we need to restore the state of the flexcom driver
on resume.
Signed-off-by: Romain Izard
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
Changes in v6:
* Reword the patch title and description
* Rename the internal structure to ddata
drivers/mfd/atmel-flexcom.c | 63 ++---
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..f684a93a3340 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,43 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *ddata;
int err;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ ddata = devm_kzalloc(>dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, ddata);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (ddata->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ ddata->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ ddata->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(ddata->base))
+ return PTR_ERR(ddata->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ ddata->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(ddata->clk))
+ return PTR_ERR(ddata->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(ddata->clk);
if (err)
return err;
@@ -76,9 +85,9 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ writel(FLEX_MR_OPMODE(ddata->opmode), ddata->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(ddata->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +98,34 @@ static const struct of_device_id atmel_flexcom_of_match[] =
{
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *ddata = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(ddata->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(ddata->opmode),
+ writel(val, ddata->base + FLEX_MR);
+
+ clk_disable_unprepare(ddata->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.14.1
[PATCH v6 1/3] clk: at91: pmc: Wait for clocks when resuming
Wait for the syncronization of all clocks when resuming, not only the
UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
when interrupts are masked, which is the case in here.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 775af473fe11..5c2b26de303e 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -107,10 +107,20 @@ static int pmc_suspend(void)
return 0;
}
+static bool pmc_ready(unsigned int mask)
+{
+ unsigned int status;
+
+ regmap_read(pmcreg, AT91_PMC_SR, );
+
+ return ((status & mask) == mask) ? 1 : 0;
+}
+
static void pmc_resume(void)
{
- int i, ret = 0;
+ int i;
u32 tmp;
+ u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
regmap_read(pmcreg, AT91_PMC_MCKR, );
if (pmc_cache.mckr != tmp)
@@ -134,13 +144,11 @@ static void pmc_resume(void)
AT91_PMC_PCR_CMD);
}
- if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
- ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
- !(tmp & AT91_PMC_LOCKU),
- 10, 5000);
- if (ret)
- pr_crit("USB PLL didn't lock when resuming\n");
- }
+ if (pmc_cache.uckr & AT91_PMC_UPLLEN)
+ mask |= AT91_PMC_LOCKU;
+
+ while (!pmc_ready(mask))
+ cpu_relax();
}
static struct syscore_ops pmc_syscore_ops = {
--
2.14.1
[PATCH v6 1/3] clk: at91: pmc: Wait for clocks when resuming
Wait for the syncronization of all clocks when resuming, not only the
UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
when interrupts are masked, which is the case in here.
Signed-off-by: Romain Izard
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
---
drivers/clk/at91/pmc.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 775af473fe11..5c2b26de303e 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -107,10 +107,20 @@ static int pmc_suspend(void)
return 0;
}
+static bool pmc_ready(unsigned int mask)
+{
+ unsigned int status;
+
+ regmap_read(pmcreg, AT91_PMC_SR, );
+
+ return ((status & mask) == mask) ? 1 : 0;
+}
+
static void pmc_resume(void)
{
- int i, ret = 0;
+ int i;
u32 tmp;
+ u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
regmap_read(pmcreg, AT91_PMC_MCKR, );
if (pmc_cache.mckr != tmp)
@@ -134,13 +144,11 @@ static void pmc_resume(void)
AT91_PMC_PCR_CMD);
}
- if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
- ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
- !(tmp & AT91_PMC_LOCKU),
- 10, 5000);
- if (ret)
- pr_crit("USB PLL didn't lock when resuming\n");
- }
+ if (pmc_cache.uckr & AT91_PMC_UPLLEN)
+ mask |= AT91_PMC_LOCKU;
+
+ while (!pmc_ready(mask))
+ cpu_relax();
}
static struct syscore_ops pmc_syscore_ops = {
--
2.14.1
[PATCH v6 3/3] clk: at91: pmc: Support backup for programmable clocks
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
Changes in v6:
* Use the correct author email address
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.14.1
[PATCH v6 3/3] clk: at91: pmc: Support backup for programmable clocks
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
Changes in v6:
* Use the correct author email address
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.14.1
[PATCH v6 0/3] Clock patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the proposed version has been running the same test for more than a month without hanging. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Changes in v5: * drop all patches already taken * split the patch series by subsystem Changes in v6: * rebase on v4.15-rc3 Romain Izard (3): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks drivers/clk/at91/clk-programmable.c | 2 ++ drivers/clk/at91/pmc.c | 63 +++-- drivers/clk/at91/pmc.h | 2 ++ 3 files changed, 57 insertions(+), 10 deletions(-) -- 2.14.1
[PATCH v6 0/3] Clock patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the proposed version has been running the same test for more than a month without hanging. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Changes in v5: * drop all patches already taken * split the patch series by subsystem Changes in v6: * rebase on v4.15-rc3 Romain Izard (3): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks drivers/clk/at91/clk-programmable.c | 2 ++ drivers/clk/at91/pmc.c | 63 +++-- drivers/clk/at91/pmc.h | 2 ++ 3 files changed, 57 insertions(+), 10 deletions(-) -- 2.14.1
[PATCH v6 2/3] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.14.1
[PATCH v6 2/3] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.14.1
[PATCH v2] clocksource: tcb_clksrc: Fix clock speed message
The clock speed displayed at boot in an information message was 500 kHz
too high compared to its real value. As the value is not used anywhere,
there is no functional impact.
Fix the rounding formula to display the correct value.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
v2: rebase over v4.15-rc1
There is no specified maintainer for this file, only supporters.
Nicolas, could you pick this through the at91 tree as the TCB block
is an AT91 peripheral ?
drivers/clocksource/tcb_clksrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 9de47d4d2d9e..43f4d5c4d6fa 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -384,7 +384,7 @@ static int __init tcb_clksrc_init(void)
printk(bootinfo, clksrc.name, CONFIG_ATMEL_TCB_CLKSRC_BLOCK,
divided_rate / 100,
- ((divided_rate + 50) % 100) / 1000);
+ ((divided_rate % 100) + 500) / 1000);
if (tc->tcb_config && tc->tcb_config->counter_width == 32) {
/* use apropriate function to read 32 bit counter */
--
2.14.1
[PATCH v2] clocksource: tcb_clksrc: Fix clock speed message
The clock speed displayed at boot in an information message was 500 kHz
too high compared to its real value. As the value is not used anywhere,
there is no functional impact.
Fix the rounding formula to display the correct value.
Signed-off-by: Romain Izard
---
v2: rebase over v4.15-rc1
There is no specified maintainer for this file, only supporters.
Nicolas, could you pick this through the at91 tree as the TCB block
is an AT91 peripheral ?
drivers/clocksource/tcb_clksrc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index 9de47d4d2d9e..43f4d5c4d6fa 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -384,7 +384,7 @@ static int __init tcb_clksrc_init(void)
printk(bootinfo, clksrc.name, CONFIG_ATMEL_TCB_CLKSRC_BLOCK,
divided_rate / 100,
- ((divided_rate + 50) % 100) / 1000);
+ ((divided_rate % 100) + 500) / 1000);
if (tc->tcb_config && tc->tcb_config->counter_width == 32) {
/* use apropriate function to read 32 bit counter */
--
2.14.1
Re: [PATCH 2/2] crypto: atmel-aes - Reset the controller before each use
2017-11-06 16:45 GMT+01:00 Tudor Ambarus <tudor.amba...@microchip.com>: > Hi, Romain, > > On 10/31/2017 05:25 PM, Romain Izard wrote: >> >> When using the rfc4543(gcm(aes))) mode, the registers of the hardware >> engine are not empty after use. If the engine is not reset before its >> next use, the following results will be invalid. >> >> Always reset the hardware engine. > > > Thanks for the fix! I could reproduce the issue only when running > rfc4543(gcm(aes))) and then, immediately after, ecb(aes). > > Have you encountered this bug with other combination of algorithms? > > I'm trying to isolate the bug so that we can have a more fine-grained > fix. I just ran the tcrypt tests because they were failing on the cts(cbc(aes)) transform and I observed this issue when the ecb test failed only on the second run. For me, the issue looks like the rfc4543 mode does not read all the registers from the AES engine, and the following operation fails because the registers are reused directly in the ECB mode. As the ECB mode is a rare case where we do not use an IV, this may be the reason why other modes do not display the issue. -- Romain Izard
Re: [PATCH 2/2] crypto: atmel-aes - Reset the controller before each use
2017-11-06 16:45 GMT+01:00 Tudor Ambarus : > Hi, Romain, > > On 10/31/2017 05:25 PM, Romain Izard wrote: >> >> When using the rfc4543(gcm(aes))) mode, the registers of the hardware >> engine are not empty after use. If the engine is not reset before its >> next use, the following results will be invalid. >> >> Always reset the hardware engine. > > > Thanks for the fix! I could reproduce the issue only when running > rfc4543(gcm(aes))) and then, immediately after, ecb(aes). > > Have you encountered this bug with other combination of algorithms? > > I'm trying to isolate the bug so that we can have a more fine-grained > fix. I just ran the tcrypt tests because they were failing on the cts(cbc(aes)) transform and I observed this issue when the ecb test failed only on the second run. For me, the issue looks like the rfc4543 mode does not read all the registers from the AES engine, and the following operation fails because the registers are reused directly in the ECB mode. As the ECB mode is a rare case where we do not use an IV, this may be the reason why other modes do not display the issue. -- Romain Izard
[PATCH 1/2] crypto: atmel-aes - properly set IV after {en,de}crypt
Certain cipher modes like CTS expect the IV (req->info) of
ablkcipher_request (or equivalently req->iv of skcipher_request) to
contain the last ciphertext block when the {en,de}crypt operation is done.
Fix this issue for the Atmel AES hardware engine. The tcrypt test
case for cts(cbc(aes)) is now correctly passed.
In the case of in-place decryption, copy the ciphertext in an
intermediate buffer before decryption.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/crypto/atmel-aes.c | 40 +---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 29e20c37f3a6..53432ab97d7e 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -110,6 +110,7 @@ struct atmel_aes_base_ctx {
int keylen;
u32 key[AES_KEYSIZE_256 / sizeof(u32)];
u16 block_size;
+ boolis_aead;
};
struct atmel_aes_ctx {
@@ -156,6 +157,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
+ u32 lastc[AES_BLOCK_SIZE / sizeof(u32)];
};
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
@@ -497,12 +499,34 @@ static void atmel_aes_authenc_complete(struct
atmel_aes_dev *dd, int err);
static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
{
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
- atmel_aes_authenc_complete(dd, err);
+ if (dd->ctx->is_aead)
+ atmel_aes_authenc_complete(dd, err);
#endif
clk_disable(dd->iclk);
dd->flags &= ~AES_FLAGS_BUSY;
+ if (!dd->ctx->is_aead) {
+ struct ablkcipher_request *req =
+ ablkcipher_request_cast(dd->areq);
+ struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ struct crypto_ablkcipher *ablkcipher =
+ crypto_ablkcipher_reqtfm(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
+ if (rctx->mode & AES_FLAGS_ENCRYPT) {
+ scatterwalk_map_and_copy(req->info, req->dst,
+ req->nbytes - ivsize, ivsize, 0);
+ } else {
+ if (req->src == req->dst) {
+ memcpy(req->info, rctx->lastc, ivsize);
+ } else {
+ scatterwalk_map_and_copy(req->info, req->src,
+ req->nbytes - ivsize, ivsize, 0);
+ }
+ }
+ }
+
if (dd->is_async)
dd->areq->complete(dd->areq, err);
@@ -1071,11 +1095,11 @@ static int atmel_aes_ctr_start(struct atmel_aes_dev *dd)
static int atmel_aes_crypt(struct ablkcipher_request *req, unsigned long mode)
{
- struct atmel_aes_base_ctx *ctx;
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
+ struct atmel_aes_base_ctx *ctx = crypto_ablkcipher_ctx(ablkcipher);
struct atmel_aes_reqctx *rctx;
struct atmel_aes_dev *dd;
- ctx = crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));
switch (mode & AES_FLAGS_OPMODE_MASK) {
case AES_FLAGS_CFB8:
ctx->block_size = CFB8_BLOCK_SIZE;
@@ -1097,6 +1121,7 @@ static int atmel_aes_crypt(struct ablkcipher_request
*req, unsigned long mode)
ctx->block_size = AES_BLOCK_SIZE;
break;
}
+ ctx->is_aead = false;
dd = atmel_aes_find_dev(ctx);
if (!dd)
@@ -1105,6 +1130,13 @@ static int atmel_aes_crypt(struct ablkcipher_request
*req, unsigned long mode)
rctx = ablkcipher_request_ctx(req);
rctx->mode = mode;
+ if (!(mode & AES_FLAGS_ENCRYPT) && (req->src == req->dst)) {
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
+ scatterwalk_map_and_copy(rctx->lastc, req->src,
+ (req->nbytes - ivsize), ivsize, 0);
+ }
+
return atmel_aes_handle_queue(dd, >base);
}
@@ -1739,6 +1771,7 @@ static int atmel_aes_gcm_crypt(struct aead_request *req,
ctx = crypto_aead_ctx(crypto_aead_reqtfm(req));
ctx->block_size = AES_BLOCK_SIZE;
+ ctx->is_aead = true;
dd = atmel_aes_find_dev(ctx);
if (!dd)
@@ -2223,6 +2256,7 @@ static int atmel_aes_authenc_crypt(struct aead_request
*req,
rctx->base.mode = mode;
ctx->block_size = AES_BLOCK_SIZE;
+ ctx->is_aead = true;
dd = atmel_aes_find_dev(ctx);
if (!dd)
--
2.14.1
[PATCH 1/2] crypto: atmel-aes - properly set IV after {en,de}crypt
Certain cipher modes like CTS expect the IV (req->info) of
ablkcipher_request (or equivalently req->iv of skcipher_request) to
contain the last ciphertext block when the {en,de}crypt operation is done.
Fix this issue for the Atmel AES hardware engine. The tcrypt test
case for cts(cbc(aes)) is now correctly passed.
In the case of in-place decryption, copy the ciphertext in an
intermediate buffer before decryption.
Signed-off-by: Romain Izard
---
drivers/crypto/atmel-aes.c | 40 +---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 29e20c37f3a6..53432ab97d7e 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -110,6 +110,7 @@ struct atmel_aes_base_ctx {
int keylen;
u32 key[AES_KEYSIZE_256 / sizeof(u32)];
u16 block_size;
+ boolis_aead;
};
struct atmel_aes_ctx {
@@ -156,6 +157,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
+ u32 lastc[AES_BLOCK_SIZE / sizeof(u32)];
};
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
@@ -497,12 +499,34 @@ static void atmel_aes_authenc_complete(struct
atmel_aes_dev *dd, int err);
static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
{
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
- atmel_aes_authenc_complete(dd, err);
+ if (dd->ctx->is_aead)
+ atmel_aes_authenc_complete(dd, err);
#endif
clk_disable(dd->iclk);
dd->flags &= ~AES_FLAGS_BUSY;
+ if (!dd->ctx->is_aead) {
+ struct ablkcipher_request *req =
+ ablkcipher_request_cast(dd->areq);
+ struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ struct crypto_ablkcipher *ablkcipher =
+ crypto_ablkcipher_reqtfm(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
+ if (rctx->mode & AES_FLAGS_ENCRYPT) {
+ scatterwalk_map_and_copy(req->info, req->dst,
+ req->nbytes - ivsize, ivsize, 0);
+ } else {
+ if (req->src == req->dst) {
+ memcpy(req->info, rctx->lastc, ivsize);
+ } else {
+ scatterwalk_map_and_copy(req->info, req->src,
+ req->nbytes - ivsize, ivsize, 0);
+ }
+ }
+ }
+
if (dd->is_async)
dd->areq->complete(dd->areq, err);
@@ -1071,11 +1095,11 @@ static int atmel_aes_ctr_start(struct atmel_aes_dev *dd)
static int atmel_aes_crypt(struct ablkcipher_request *req, unsigned long mode)
{
- struct atmel_aes_base_ctx *ctx;
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
+ struct atmel_aes_base_ctx *ctx = crypto_ablkcipher_ctx(ablkcipher);
struct atmel_aes_reqctx *rctx;
struct atmel_aes_dev *dd;
- ctx = crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));
switch (mode & AES_FLAGS_OPMODE_MASK) {
case AES_FLAGS_CFB8:
ctx->block_size = CFB8_BLOCK_SIZE;
@@ -1097,6 +1121,7 @@ static int atmel_aes_crypt(struct ablkcipher_request
*req, unsigned long mode)
ctx->block_size = AES_BLOCK_SIZE;
break;
}
+ ctx->is_aead = false;
dd = atmel_aes_find_dev(ctx);
if (!dd)
@@ -1105,6 +1130,13 @@ static int atmel_aes_crypt(struct ablkcipher_request
*req, unsigned long mode)
rctx = ablkcipher_request_ctx(req);
rctx->mode = mode;
+ if (!(mode & AES_FLAGS_ENCRYPT) && (req->src == req->dst)) {
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+
+ scatterwalk_map_and_copy(rctx->lastc, req->src,
+ (req->nbytes - ivsize), ivsize, 0);
+ }
+
return atmel_aes_handle_queue(dd, >base);
}
@@ -1739,6 +1771,7 @@ static int atmel_aes_gcm_crypt(struct aead_request *req,
ctx = crypto_aead_ctx(crypto_aead_reqtfm(req));
ctx->block_size = AES_BLOCK_SIZE;
+ ctx->is_aead = true;
dd = atmel_aes_find_dev(ctx);
if (!dd)
@@ -2223,6 +2256,7 @@ static int atmel_aes_authenc_crypt(struct aead_request
*req,
rctx->base.mode = mode;
ctx->block_size = AES_BLOCK_SIZE;
+ ctx->is_aead = true;
dd = atmel_aes_find_dev(ctx);
if (!dd)
--
2.14.1
[PATCH 2/2] crypto: atmel-aes - Reset the controller before each use
When using the rfc4543(gcm(aes))) mode, the registers of the hardware
engine are not empty after use. If the engine is not reset before its
next use, the following results will be invalid.
Always reset the hardware engine.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/crypto/atmel-aes.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 53432ab97d7e..024914e82734 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -76,12 +76,11 @@
AES_FLAGS_ENCRYPT |\
AES_FLAGS_GTAGEN)
-#define AES_FLAGS_INIT BIT(2)
#define AES_FLAGS_BUSY BIT(3)
#define AES_FLAGS_DUMP_REG BIT(4)
#define AES_FLAGS_OWN_SHA BIT(5)
-#define AES_FLAGS_PERSISTENT (AES_FLAGS_INIT | AES_FLAGS_BUSY)
+#define AES_FLAGS_PERSISTENT AES_FLAGS_BUSY
#define ATMEL_AES_QUEUE_LENGTH 50
@@ -450,11 +449,8 @@ static int atmel_aes_hw_init(struct atmel_aes_dev *dd)
if (err)
return err;
- if (!(dd->flags & AES_FLAGS_INIT)) {
- atmel_aes_write(dd, AES_CR, AES_CR_SWRST);
- atmel_aes_write(dd, AES_MR, 0xE << AES_MR_CKEY_OFFSET);
- dd->flags |= AES_FLAGS_INIT;
- }
+ atmel_aes_write(dd, AES_CR, AES_CR_SWRST);
+ atmel_aes_write(dd, AES_MR, 0xE << AES_MR_CKEY_OFFSET);
return 0;
}
--
2.14.1
[PATCH 2/2] crypto: atmel-aes - Reset the controller before each use
When using the rfc4543(gcm(aes))) mode, the registers of the hardware
engine are not empty after use. If the engine is not reset before its
next use, the following results will be invalid.
Always reset the hardware engine.
Signed-off-by: Romain Izard
---
drivers/crypto/atmel-aes.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 53432ab97d7e..024914e82734 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -76,12 +76,11 @@
AES_FLAGS_ENCRYPT |\
AES_FLAGS_GTAGEN)
-#define AES_FLAGS_INIT BIT(2)
#define AES_FLAGS_BUSY BIT(3)
#define AES_FLAGS_DUMP_REG BIT(4)
#define AES_FLAGS_OWN_SHA BIT(5)
-#define AES_FLAGS_PERSISTENT (AES_FLAGS_INIT | AES_FLAGS_BUSY)
+#define AES_FLAGS_PERSISTENT AES_FLAGS_BUSY
#define ATMEL_AES_QUEUE_LENGTH 50
@@ -450,11 +449,8 @@ static int atmel_aes_hw_init(struct atmel_aes_dev *dd)
if (err)
return err;
- if (!(dd->flags & AES_FLAGS_INIT)) {
- atmel_aes_write(dd, AES_CR, AES_CR_SWRST);
- atmel_aes_write(dd, AES_MR, 0xE << AES_MR_CKEY_OFFSET);
- dd->flags |= AES_FLAGS_INIT;
- }
+ atmel_aes_write(dd, AES_CR, AES_CR_SWRST);
+ atmel_aes_write(dd, AES_MR, 0xE << AES_MR_CKEY_OFFSET);
return 0;
}
--
2.14.1
[PATCH 0/2] Fixes for the Atmel AES crypto module
After encountering an issue with cts(cbc(aes)) in the Atmel AES module,
I have used tcrypt and libkcapi's test suite to validate my fix. This led
me to observe some other issues.
This series includes the IV issue correction for the Atmel AES crypto
engine, as well as a secondary issue observed when running
'insmod tcrypt.ko mode=10' and 'insmod tcrypt.ko mode=152' on a SAMA5D2
board.
The libkcapi test suite still reports some problems, for example when the
input data is too large to fit into an intermediate buffer in unaligned
cases. And it seems that with the v4.14 updates, new asynchronous tests
are enabled and report new issues.
Romain Izard (2):
crypto: atmel-aes - properly set IV after {en,de}crypt
crypto: atmel-aes - Reset the controller before each use
drivers/crypto/atmel-aes.c | 50 --
1 file changed, 40 insertions(+), 10 deletions(-)
--
2.14.1
[PATCH 0/2] Fixes for the Atmel AES crypto module
After encountering an issue with cts(cbc(aes)) in the Atmel AES module,
I have used tcrypt and libkcapi's test suite to validate my fix. This led
me to observe some other issues.
This series includes the IV issue correction for the Atmel AES crypto
engine, as well as a secondary issue observed when running
'insmod tcrypt.ko mode=10' and 'insmod tcrypt.ko mode=152' on a SAMA5D2
board.
The libkcapi test suite still reports some problems, for example when the
input data is too large to fit into an intermediate buffer in unaligned
cases. And it seems that with the v4.14 updates, new asynchronous tests
are enabled and report new issues.
Romain Izard (2):
crypto: atmel-aes - properly set IV after {en,de}crypt
crypto: atmel-aes - Reset the controller before each use
drivers/crypto/atmel-aes.c | 50 --
1 file changed, 40 insertions(+), 10 deletions(-)
--
2.14.1
[PATCH] crypto: ccm - preserve the IV buffer
The IV buffer used during CCM operations is used twice, during both the hashing step and the ciphering step. When using a hardware accelerator that updates the contents of the IV buffer at the end of ciphering operations, the value will be modified. In the decryption case, the subsequent setup of the hashing algorithm will interpret the updated IV instead of the original value, which can lead to out-of-bounds writes. Reuse the idata buffer, only used in the hashing step, to preserve the IV's value during the ciphering step in the decryption case. Signed-off-by: Romain Izard <romain.izard@gmail.com> --- crypto/ccm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/ccm.c b/crypto/ccm.c index 1ce37ae0ce56..0a083342ec8c 100644 --- a/crypto/ccm.c +++ b/crypto/ccm.c @@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req) unsigned int cryptlen = req->cryptlen; u8 *authtag = pctx->auth_tag; u8 *odata = pctx->odata; - u8 *iv = req->iv; + u8 *iv = pctx->idata; int err; cryptlen -= authsize; @@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req) if (req->src != req->dst) dst = pctx->dst; + memcpy(iv, req->iv, 16); + skcipher_request_set_tfm(skreq, ctx->ctr); skcipher_request_set_callback(skreq, pctx->flags, crypto_ccm_decrypt_done, req); -- 2.14.1
[PATCH] crypto: ccm - preserve the IV buffer
The IV buffer used during CCM operations is used twice, during both the hashing step and the ciphering step. When using a hardware accelerator that updates the contents of the IV buffer at the end of ciphering operations, the value will be modified. In the decryption case, the subsequent setup of the hashing algorithm will interpret the updated IV instead of the original value, which can lead to out-of-bounds writes. Reuse the idata buffer, only used in the hashing step, to preserve the IV's value during the ciphering step in the decryption case. Signed-off-by: Romain Izard --- crypto/ccm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crypto/ccm.c b/crypto/ccm.c index 1ce37ae0ce56..0a083342ec8c 100644 --- a/crypto/ccm.c +++ b/crypto/ccm.c @@ -363,7 +363,7 @@ static int crypto_ccm_decrypt(struct aead_request *req) unsigned int cryptlen = req->cryptlen; u8 *authtag = pctx->auth_tag; u8 *odata = pctx->odata; - u8 *iv = req->iv; + u8 *iv = pctx->idata; int err; cryptlen -= authsize; @@ -379,6 +379,8 @@ static int crypto_ccm_decrypt(struct aead_request *req) if (req->src != req->dst) dst = pctx->dst; + memcpy(iv, req->iv, 16); + skcipher_request_set_tfm(skreq, ctx->ctr); skcipher_request_set_callback(skreq, pctx->flags, crypto_ccm_decrypt_done, req); -- 2.14.1
Re: [PATCH v5] atmel_flexcom: Support backup mode
2017-10-23 18:07 GMT+02:00 Lee Jones <lee.jo...@linaro.org>:
> On Mon, 23 Oct 2017, Lee Jones wrote:
>> On Thu, 19 Oct 2017, Romain Izard wrote:
>>
>> > The controller used by a flexcom module is configured at boot, and left
>> > alone after this. As the configuration will be lost after backup mode,
>> > restore the state of the flexcom driver on resume.
>> >
>> > Signed-off-by: Romain Izard <romain.izard@gmail.com>
>> > Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
>> > Tested-by: Nicolas Ferre <nicolas.fe...@microchip.com>
>> > ---
>> > Changes in v5:
>> > * extract from the patch series, and send as a standalone patch
>> >
>> > drivers/mfd/atmel-flexcom.c | 65
>> > ++---
>> > 1 file changed, 50 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
>> > index 064bde9cff5a..ef1235c4a179 100644
>> > --- a/drivers/mfd/atmel-flexcom.c
>> > +++ b/drivers/mfd/atmel-flexcom.c
>> > @@ -39,34 +39,44 @@
>> > #define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) &
>> > \
>> > FLEX_MR_OPMODE_MASK)
>> >
>> > +struct atmel_flexcom {
>> > + void __iomem *base;
>> > + u32 opmode;
>> > + struct clk *clk;
>> > +};
>> >
>> > static int atmel_flexcom_probe(struct platform_device *pdev)
>> > {
>> > struct device_node *np = pdev->dev.of_node;
>> > - struct clk *clk;
>> > struct resource *res;
>> > - void __iomem *base;
>> > - u32 opmode;
>> > + struct atmel_flexcom *afc;
>>
>> Nit: I'd prefer if you call this 'ddata'.
>>
>> But the concept and implementation is fine, so if you're going to
>> change it please do so and apply my:
>>
>> Acked-by: Lee Jones <lee.jo...@linaro.org>
>
> Also, 'back-up mode' isn't really a thing is it?
>
> How about "Reinstall state on resume" or similar?
>
The expression comes from the SAMA5D2's datasheet.
Other Atmel chips use a different single suspend mode with Linux, where
the SoC remains completely powered with a slow clock. The registers are
preserved in this mode, so there is no need for a specific suspend and
resume code.
The SoC can also be powered down, but the CPU is reset and only a small
part is powered with a backup battery to maintain a valid RTC and a
small internal SRAM.
In the SAMA5D2, the mode with only the backup power supply has been
extended to isolate the memory I/O lines, making it possible to keep the
external SDRAM memory in self-refresh. This mode has a lower consumption
compared to the slow clock mode, but it has a higher wakeup latency, and
needs specific software support in the bootloader and the kernel.
As a result, the "backup mode" expression is used to contrast with the
"slow clock" expression when describing the different suspend modes
supported by the chip.
But if you think that it is necessary, I can reword the commit.
Best regards,
--
Romain Izard
Re: [PATCH v5] atmel_flexcom: Support backup mode
2017-10-23 18:07 GMT+02:00 Lee Jones :
> On Mon, 23 Oct 2017, Lee Jones wrote:
>> On Thu, 19 Oct 2017, Romain Izard wrote:
>>
>> > The controller used by a flexcom module is configured at boot, and left
>> > alone after this. As the configuration will be lost after backup mode,
>> > restore the state of the flexcom driver on resume.
>> >
>> > Signed-off-by: Romain Izard
>> > Acked-by: Nicolas Ferre
>> > Tested-by: Nicolas Ferre
>> > ---
>> > Changes in v5:
>> > * extract from the patch series, and send as a standalone patch
>> >
>> > drivers/mfd/atmel-flexcom.c | 65
>> > ++---
>> > 1 file changed, 50 insertions(+), 15 deletions(-)
>> >
>> > diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
>> > index 064bde9cff5a..ef1235c4a179 100644
>> > --- a/drivers/mfd/atmel-flexcom.c
>> > +++ b/drivers/mfd/atmel-flexcom.c
>> > @@ -39,34 +39,44 @@
>> > #define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) &
>> > \
>> > FLEX_MR_OPMODE_MASK)
>> >
>> > +struct atmel_flexcom {
>> > + void __iomem *base;
>> > + u32 opmode;
>> > + struct clk *clk;
>> > +};
>> >
>> > static int atmel_flexcom_probe(struct platform_device *pdev)
>> > {
>> > struct device_node *np = pdev->dev.of_node;
>> > - struct clk *clk;
>> > struct resource *res;
>> > - void __iomem *base;
>> > - u32 opmode;
>> > + struct atmel_flexcom *afc;
>>
>> Nit: I'd prefer if you call this 'ddata'.
>>
>> But the concept and implementation is fine, so if you're going to
>> change it please do so and apply my:
>>
>> Acked-by: Lee Jones
>
> Also, 'back-up mode' isn't really a thing is it?
>
> How about "Reinstall state on resume" or similar?
>
The expression comes from the SAMA5D2's datasheet.
Other Atmel chips use a different single suspend mode with Linux, where
the SoC remains completely powered with a slow clock. The registers are
preserved in this mode, so there is no need for a specific suspend and
resume code.
The SoC can also be powered down, but the CPU is reset and only a small
part is powered with a backup battery to maintain a valid RTC and a
small internal SRAM.
In the SAMA5D2, the mode with only the backup power supply has been
extended to isolate the memory I/O lines, making it possible to keep the
external SDRAM memory in self-refresh. This mode has a lower consumption
compared to the slow clock mode, but it has a higher wakeup latency, and
needs specific software support in the bootloader and the kernel.
As a result, the "backup mode" expression is used to contrast with the
"slow clock" expression when describing the different suspend modes
supported by the chip.
But if you think that it is necessary, I can reword the commit.
Best regards,
--
Romain Izard
Re: [PATCH] ARM: add a private asm/unaligned.h
2017-10-20 22:01 GMT+02:00 Arnd Bergmann <a...@arndb.de>:
> The asm-generic/unaligned.h header provides two different implementations
> for accessing unaligned variables: the access_ok.h version used when
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set pretends that all pointers
> are in fact aligned, while the le_struct.h version convinces gcc that the
> alignment of a pointer is '1', to make it issue the correct load/store
> instructions depending on the architecture flags.
>
> On ARMv5 and older, we always use the second version, to let the compiler
> use byte accesses. On ARMv6 and newer, we currently use the access_ok.h
> version, so the compiler can use any instruction including stm/ldm and
> ldrd/strd that will cause an alignment trap. This trap can significantly
> impact performance when we have to do a lot of fixups and, worse, has
> led to crashes in the LZ4 decompressor code that does not have a trap
> handler.
>
> This adds an ARM specific version of asm/unaligned.h that uses the
> le_struct.h/be_struct.h implementation unconditionally. This should lead
> to essentially the same code on ARMv6+ as before, with the exception of
> using regular load/store instructions instead of the trapping instructions
> multi-register variants.
>
> The crash in the LZ4 decompressor code was probably introduced by the
> patch replacing the LZ4 implementation, commit 4e1a33b105dd ("lib: update
> LZ4 compressor module"), so linux-4.11 and higher would be affected most.
> However, we probably want to have this backported to all older stable
> kernels as well, to help with the performance issues.
>
> There are two follow-ups that I think we should also work on, but not
> backport to stable kernels, first to change the asm-generic version of
> the header to remove the ARM special case, and second to review all
> other uses of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to see if they
> might be affected by the same problem on ARM.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
Tested-by: Romain.Izard <romain.izard@gmail.com>
Without this patch, the LZ4 decompression code is invalid in v4.14-rc6,
when compiled with arm-linux-gnueabihf-gcc, version 7.2. The zImage does
not start.
With this patch, the decompression code is valid, and the kernel boots
without a problem. No fixups are reported in /proc/cpu/alignment.
--
Romain Izard
Re: [PATCH] ARM: add a private asm/unaligned.h
2017-10-20 22:01 GMT+02:00 Arnd Bergmann :
> The asm-generic/unaligned.h header provides two different implementations
> for accessing unaligned variables: the access_ok.h version used when
> CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is set pretends that all pointers
> are in fact aligned, while the le_struct.h version convinces gcc that the
> alignment of a pointer is '1', to make it issue the correct load/store
> instructions depending on the architecture flags.
>
> On ARMv5 and older, we always use the second version, to let the compiler
> use byte accesses. On ARMv6 and newer, we currently use the access_ok.h
> version, so the compiler can use any instruction including stm/ldm and
> ldrd/strd that will cause an alignment trap. This trap can significantly
> impact performance when we have to do a lot of fixups and, worse, has
> led to crashes in the LZ4 decompressor code that does not have a trap
> handler.
>
> This adds an ARM specific version of asm/unaligned.h that uses the
> le_struct.h/be_struct.h implementation unconditionally. This should lead
> to essentially the same code on ARMv6+ as before, with the exception of
> using regular load/store instructions instead of the trapping instructions
> multi-register variants.
>
> The crash in the LZ4 decompressor code was probably introduced by the
> patch replacing the LZ4 implementation, commit 4e1a33b105dd ("lib: update
> LZ4 compressor module"), so linux-4.11 and higher would be affected most.
> However, we probably want to have this backported to all older stable
> kernels as well, to help with the performance issues.
>
> There are two follow-ups that I think we should also work on, but not
> backport to stable kernels, first to change the asm-generic version of
> the header to remove the ARM special case, and second to review all
> other uses of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to see if they
> might be affected by the same problem on ARM.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arnd Bergmann
Tested-by: Romain.Izard
Without this patch, the LZ4 decompression code is invalid in v4.14-rc6,
when compiled with arm-linux-gnueabihf-gcc, version 7.2. The zImage does
not start.
With this patch, the decompression code is valid, and the kernel boots
without a problem. No fixups are reported in /proc/cpu/alignment.
--
Romain Izard
[PATCH v5] atmel_flexcom: Support backup mode
The controller used by a flexcom module is configured at boot, and left
alone after this. As the configuration will be lost after backup mode,
restore the state of the flexcom driver on resume.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Tested-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
drivers/mfd/atmel-flexcom.c | 65 ++---
1 file changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..ef1235c4a179 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,44 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *afc;
int err;
+ u32 val;
+
+ afc = devm_kzalloc(>dev, sizeof(*afc), GFP_KERNEL);
+ if (!afc)
+ return -ENOMEM;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ platform_set_drvdata(pdev, afc);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (afc->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ afc->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ afc->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(afc->base))
+ return PTR_ERR(afc->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ afc->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(afc->clk))
+ return PTR_ERR(afc->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(afc->clk);
if (err)
return err;
@@ -76,9 +86,10 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ val = FLEX_MR_OPMODE(afc->opmode);
+ writel(val, afc->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(afc->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +100,34 @@ static const struct of_device_id atmel_flexcom_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *afc = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(afc->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(afc->opmode),
+ writel(val, afc->base + FLEX_MR);
+
+ clk_disable_unprepare(afc->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.11.0
[PATCH v5] atmel_flexcom: Support backup mode
The controller used by a flexcom module is configured at boot, and left
alone after this. As the configuration will be lost after backup mode,
restore the state of the flexcom driver on resume.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
Tested-by: Nicolas Ferre
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
drivers/mfd/atmel-flexcom.c | 65 ++---
1 file changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..ef1235c4a179 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,44 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *afc;
int err;
+ u32 val;
+
+ afc = devm_kzalloc(>dev, sizeof(*afc), GFP_KERNEL);
+ if (!afc)
+ return -ENOMEM;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ platform_set_drvdata(pdev, afc);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (afc->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ afc->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ afc->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(afc->base))
+ return PTR_ERR(afc->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ afc->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(afc->clk))
+ return PTR_ERR(afc->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(afc->clk);
if (err)
return err;
@@ -76,9 +86,10 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ val = FLEX_MR_OPMODE(afc->opmode);
+ writel(val, afc->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(afc->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +100,34 @@ static const struct of_device_id atmel_flexcom_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *afc = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(afc->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(afc->opmode),
+ writel(val, afc->base + FLEX_MR);
+
+ clk_disable_unprepare(afc->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.11.0
[PATCH v5] pwm: atmel-tcb: Support backup mode
Save and restore registers for the PWM on suspend and resume, which
makes hibernation and backup modes possible.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
drivers/pwm/pwm-atmel-tcb.c | 63 +++--
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 75db585a2a94..acd3ce8ecf3f 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -37,11 +37,20 @@ struct atmel_tcb_pwm_device {
unsigned period;/* PWM period expressed in clk cycles */
};
+struct atmel_tcb_channel {
+ u32 enabled;
+ u32 cmr;
+ u32 ra;
+ u32 rb;
+ u32 rc;
+};
+
struct atmel_tcb_pwm_chip {
struct pwm_chip chip;
spinlock_t lock;
struct atmel_tc *tc;
struct atmel_tcb_pwm_device *pwms[NPWM];
+ struct atmel_tcb_channel bkup[NPWM / 2];
};
static inline struct atmel_tcb_pwm_chip *to_tcb_chip(struct pwm_chip *chip)
@@ -175,12 +184,15 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip,
struct pwm_device *pwm)
* Use software trigger to apply the new setting.
* If both PWM devices in this group are disabled we stop the clock.
*/
- if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC)))
+ if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC))) {
__raw_writel(ATMEL_TC_SWTRG | ATMEL_TC_CLKDIS,
regs + ATMEL_TC_REG(group, CCR));
- else
+ tcbpwmc->bkup[group].enabled = 1;
+ } else {
__raw_writel(ATMEL_TC_SWTRG, regs +
ATMEL_TC_REG(group, CCR));
+ tcbpwmc->bkup[group].enabled = 0;
+ }
spin_unlock(>lock);
}
@@ -263,6 +275,7 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip,
struct pwm_device *pwm)
/* Use software trigger to apply the new setting */
__raw_writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
regs + ATMEL_TC_REG(group, CCR));
+ tcbpwmc->bkup[group].enabled = 1;
spin_unlock(>lock);
return 0;
}
@@ -445,10 +458,56 @@ static const struct of_device_id atmel_tcb_pwm_dt_ids[] =
{
};
MODULE_DEVICE_TABLE(of, atmel_tcb_pwm_dt_ids);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_tcb_pwm_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
+ void __iomem *base = tcbpwm->tc->regs;
+ int i;
+
+ for (i = 0; i < (NPWM / 2); i++) {
+ struct atmel_tcb_channel *chan = >bkup[i];
+
+ chan->cmr = readl(base + ATMEL_TC_REG(i, CMR));
+ chan->ra = readl(base + ATMEL_TC_REG(i, RA));
+ chan->rb = readl(base + ATMEL_TC_REG(i, RB));
+ chan->rc = readl(base + ATMEL_TC_REG(i, RC));
+ }
+ return 0;
+}
+
+static int atmel_tcb_pwm_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
+ void __iomem *base = tcbpwm->tc->regs;
+ int i;
+
+ for (i = 0; i < (NPWM / 2); i++) {
+ struct atmel_tcb_channel *chan = >bkup[i];
+
+ writel(chan->cmr, base + ATMEL_TC_REG(i, CMR));
+ writel(chan->ra, base + ATMEL_TC_REG(i, RA));
+ writel(chan->rb, base + ATMEL_TC_REG(i, RB));
+ writel(chan->rc, base + ATMEL_TC_REG(i, RC));
+ if (chan->enabled) {
+ writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
+ base + ATMEL_TC_REG(i, CCR));
+ }
+ }
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_tcb_pwm_pm_ops, atmel_tcb_pwm_suspend,
+atmel_tcb_pwm_resume);
+
static struct platform_driver atmel_tcb_pwm_driver = {
.driver = {
.name = "atmel-tcb-pwm",
.of_match_table = atmel_tcb_pwm_dt_ids,
+ .pm = _tcb_pwm_pm_ops,
},
.probe = atmel_tcb_pwm_probe,
.remove = atmel_tcb_pwm_remove,
--
2.11.0
[PATCH v5] pwm: atmel-tcb: Support backup mode
Save and restore registers for the PWM on suspend and resume, which
makes hibernation and backup modes possible.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
Changes in v5:
* extract from the patch series, and send as a standalone patch
drivers/pwm/pwm-atmel-tcb.c | 63 +++--
1 file changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-atmel-tcb.c b/drivers/pwm/pwm-atmel-tcb.c
index 75db585a2a94..acd3ce8ecf3f 100644
--- a/drivers/pwm/pwm-atmel-tcb.c
+++ b/drivers/pwm/pwm-atmel-tcb.c
@@ -37,11 +37,20 @@ struct atmel_tcb_pwm_device {
unsigned period;/* PWM period expressed in clk cycles */
};
+struct atmel_tcb_channel {
+ u32 enabled;
+ u32 cmr;
+ u32 ra;
+ u32 rb;
+ u32 rc;
+};
+
struct atmel_tcb_pwm_chip {
struct pwm_chip chip;
spinlock_t lock;
struct atmel_tc *tc;
struct atmel_tcb_pwm_device *pwms[NPWM];
+ struct atmel_tcb_channel bkup[NPWM / 2];
};
static inline struct atmel_tcb_pwm_chip *to_tcb_chip(struct pwm_chip *chip)
@@ -175,12 +184,15 @@ static void atmel_tcb_pwm_disable(struct pwm_chip *chip,
struct pwm_device *pwm)
* Use software trigger to apply the new setting.
* If both PWM devices in this group are disabled we stop the clock.
*/
- if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC)))
+ if (!(cmr & (ATMEL_TC_ACPC | ATMEL_TC_BCPC))) {
__raw_writel(ATMEL_TC_SWTRG | ATMEL_TC_CLKDIS,
regs + ATMEL_TC_REG(group, CCR));
- else
+ tcbpwmc->bkup[group].enabled = 1;
+ } else {
__raw_writel(ATMEL_TC_SWTRG, regs +
ATMEL_TC_REG(group, CCR));
+ tcbpwmc->bkup[group].enabled = 0;
+ }
spin_unlock(>lock);
}
@@ -263,6 +275,7 @@ static int atmel_tcb_pwm_enable(struct pwm_chip *chip,
struct pwm_device *pwm)
/* Use software trigger to apply the new setting */
__raw_writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
regs + ATMEL_TC_REG(group, CCR));
+ tcbpwmc->bkup[group].enabled = 1;
spin_unlock(>lock);
return 0;
}
@@ -445,10 +458,56 @@ static const struct of_device_id atmel_tcb_pwm_dt_ids[] =
{
};
MODULE_DEVICE_TABLE(of, atmel_tcb_pwm_dt_ids);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_tcb_pwm_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
+ void __iomem *base = tcbpwm->tc->regs;
+ int i;
+
+ for (i = 0; i < (NPWM / 2); i++) {
+ struct atmel_tcb_channel *chan = >bkup[i];
+
+ chan->cmr = readl(base + ATMEL_TC_REG(i, CMR));
+ chan->ra = readl(base + ATMEL_TC_REG(i, RA));
+ chan->rb = readl(base + ATMEL_TC_REG(i, RB));
+ chan->rc = readl(base + ATMEL_TC_REG(i, RC));
+ }
+ return 0;
+}
+
+static int atmel_tcb_pwm_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct atmel_tcb_pwm_chip *tcbpwm = platform_get_drvdata(pdev);
+ void __iomem *base = tcbpwm->tc->regs;
+ int i;
+
+ for (i = 0; i < (NPWM / 2); i++) {
+ struct atmel_tcb_channel *chan = >bkup[i];
+
+ writel(chan->cmr, base + ATMEL_TC_REG(i, CMR));
+ writel(chan->ra, base + ATMEL_TC_REG(i, RA));
+ writel(chan->rb, base + ATMEL_TC_REG(i, RB));
+ writel(chan->rc, base + ATMEL_TC_REG(i, RC));
+ if (chan->enabled) {
+ writel(ATMEL_TC_CLKEN | ATMEL_TC_SWTRG,
+ base + ATMEL_TC_REG(i, CCR));
+ }
+ }
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_tcb_pwm_pm_ops, atmel_tcb_pwm_suspend,
+atmel_tcb_pwm_resume);
+
static struct platform_driver atmel_tcb_pwm_driver = {
.driver = {
.name = "atmel-tcb-pwm",
.of_match_table = atmel_tcb_pwm_dt_ids,
+ .pm = _tcb_pwm_pm_ops,
},
.probe = atmel_tcb_pwm_probe,
.remove = atmel_tcb_pwm_remove,
--
2.11.0
[PATCH v5 3/3] clk: at91: pmc: Support backup for programmable clocks
From: Romain Izard <romain.iz...@mobile-devices.fr>
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.11.0
[PATCH v5 2/3] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.11.0
[PATCH v5 3/3] clk: at91: pmc: Support backup for programmable clocks
From: Romain Izard
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.11.0
[PATCH v5 2/3] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.11.0
[PATCH v5 1/3] clk: at91: pmc: Wait for clocks when resuming
Wait for the syncronization of all clocks when resuming, not only the
UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
when interrupts are masked, which is the case in here.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 775af473fe11..5c2b26de303e 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -107,10 +107,20 @@ static int pmc_suspend(void)
return 0;
}
+static bool pmc_ready(unsigned int mask)
+{
+ unsigned int status;
+
+ regmap_read(pmcreg, AT91_PMC_SR, );
+
+ return ((status & mask) == mask) ? 1 : 0;
+}
+
static void pmc_resume(void)
{
- int i, ret = 0;
+ int i;
u32 tmp;
+ u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
regmap_read(pmcreg, AT91_PMC_MCKR, );
if (pmc_cache.mckr != tmp)
@@ -134,13 +144,11 @@ static void pmc_resume(void)
AT91_PMC_PCR_CMD);
}
- if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
- ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
- !(tmp & AT91_PMC_LOCKU),
- 10, 5000);
- if (ret)
- pr_crit("USB PLL didn't lock when resuming\n");
- }
+ if (pmc_cache.uckr & AT91_PMC_UPLLEN)
+ mask |= AT91_PMC_LOCKU;
+
+ while (!pmc_ready(mask))
+ cpu_relax();
}
static struct syscore_ops pmc_syscore_ops = {
--
2.11.0
[PATCH v5 1/3] clk: at91: pmc: Wait for clocks when resuming
Wait for the syncronization of all clocks when resuming, not only the
UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
when interrupts are masked, which is the case in here.
Signed-off-by: Romain Izard
Acked-by: Ludovic Desroches
Acked-by: Nicolas Ferre
---
drivers/clk/at91/pmc.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 775af473fe11..5c2b26de303e 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -107,10 +107,20 @@ static int pmc_suspend(void)
return 0;
}
+static bool pmc_ready(unsigned int mask)
+{
+ unsigned int status;
+
+ regmap_read(pmcreg, AT91_PMC_SR, );
+
+ return ((status & mask) == mask) ? 1 : 0;
+}
+
static void pmc_resume(void)
{
- int i, ret = 0;
+ int i;
u32 tmp;
+ u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
regmap_read(pmcreg, AT91_PMC_MCKR, );
if (pmc_cache.mckr != tmp)
@@ -134,13 +144,11 @@ static void pmc_resume(void)
AT91_PMC_PCR_CMD);
}
- if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
- ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
- !(tmp & AT91_PMC_LOCKU),
- 10, 5000);
- if (ret)
- pr_crit("USB PLL didn't lock when resuming\n");
- }
+ if (pmc_cache.uckr & AT91_PMC_UPLLEN)
+ mask |= AT91_PMC_LOCKU;
+
+ while (!pmc_ready(mask))
+ cpu_relax();
}
static struct syscore_ops pmc_syscore_ops = {
--
2.11.0
[PATCH v5 0/3] Clock patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the prototype version has been running the same test for more than a week without hanging. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Changes in v5: * drop all patches already taken * split the patch series by subsystem Romain Izard (8): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks drivers/clk/at91/clk-programmable.c | 2 ++ drivers/clk/at91/pmc.c | 63 +++-- drivers/clk/at91/pmc.h | 2 ++ 3 files changed, 57 insertions(+), 10 deletions(-) -- 2.11.0
[PATCH v5 0/3] Clock patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the prototype version has been running the same test for more than a week without hanging. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Changes in v5: * drop all patches already taken * split the patch series by subsystem Romain Izard (8): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks drivers/clk/at91/clk-programmable.c | 2 ++ drivers/clk/at91/pmc.c | 63 +++-- drivers/clk/at91/pmc.h | 2 ++ 3 files changed, 57 insertions(+), 10 deletions(-) -- 2.11.0
Re: [PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
2017-10-10 16:16 GMT+02:00 Boris Brezillon <boris.brezil...@free-electrons.com>: > Hi Romain, > > May I ask why you're sending this patch to the MTD ML? > This is related to an issue reported by Nicolas Feignon with UBIFS and fscrypt, which was reported on both mtd and fscrypt mailing lists. The file names are encrypted with cts(cbc(aes)), and this patch tried to fix a SAMA5D2 issue where the file names are not correctly encrypted when hardware acceleration is enabled. > While I'm here, can you have a look at this patch [1] and add you > Reviewed-by/Tested-by? > > [1]http://patchwork.ozlabs.org/patch/821959/ I'll try it. -- Romain Izard
Re: [PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
2017-10-10 16:16 GMT+02:00 Boris Brezillon : > Hi Romain, > > May I ask why you're sending this patch to the MTD ML? > This is related to an issue reported by Nicolas Feignon with UBIFS and fscrypt, which was reported on both mtd and fscrypt mailing lists. The file names are encrypted with cts(cbc(aes)), and this patch tried to fix a SAMA5D2 issue where the file names are not correctly encrypted when hardware acceleration is enabled. > While I'm here, can you have a look at this patch [1] and add you > Reviewed-by/Tested-by? > > [1]http://patchwork.ozlabs.org/patch/821959/ I'll try it. -- Romain Izard
Re: [PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
2017-10-06 17:51 GMT+02:00 Romain Izard <romain.izard@gmail.com>:
>
> Certain cipher modes like CTS expect the IV (req->info) of
> ablkcipher_request (or equivalently req->iv of skcipher_request) to
> contain the last ciphertext block when the {en,de}crypt operation is done.
>
> Fix this issue for the Atmel AES hardware engine. The tcrypt test
> case for cts(cbc(aes)) is now correctly passed.
>
> To handle the case of in-place decryption, copy the ciphertext in an
> intermediate buffer before decryption.
>
Unfortunately this does not seem to be enough. The tcrypt module's tests
pass, but I encounter more issues. If I run the libkcapi test suite, I
end up randomly with the following type of panic:
8< --
Unable to handle kernel paging request at virtual address 7ffc
pgd = dee9c000
[7ffc] *pgd=
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 2187 Comm: kcapi Not tainted 4.13.4+ #16
Hardware name: Atmel SAMA5
task: dec7f280 task.stack: dee82000
PC is at memcpy+0x114/0x330
LR is at atmel_aes_transfer_complete+0x64/0xe8
pc : []lr : []psr: 2013
sp : dee83bcc ip : 0003 fp : dee83bfc
r10: r9 : df638940 r8 : df638874
r7 : 0010 r6 : r5 : df638940 r4 : dec68110
r3 : 4004 r2 : 000c r1 : 7ffc r0 : df638afc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c53c7d Table: 3ee9c059 DAC: 0051
Process kcapi (pid: 2187, stack limit = 0xdee82208)
Stack: (0xdee83bcc to 0xdee84000)
3bc0:df638afc dec68110 c05e419c
3be0: 0030 dec68110 df557040 0030 dee83c3c dee83c00 c05e61cc c05e4144
3c00: 10031000 dec68110 df6388a4 0030 df6388a4 dec68110 df6388a4 0030
3c20: 0030 df638874 df557070 dee83c6c dee83c40 c05e488c c05e6064
3c40: df6388a4 df638874 dee83c6c dec68110 0030 0030 df6388a4 df638874
3c60: dee83c94 dee83c70 c05e4998 c05e471c 0030 dec68110 df557040
3c80: df638874 df557070 dee83cd4 dee83c98 c05e6198 c05e48d4 c05e6058
3ca0: dee83cbc df6388a4 c041ab04 dec68110 df557040 df638940 ff8d
3cc0: a013 0004 dee83d04 dee83cd8 c05e62f8 c05e6064 dee83d3c dee83ce8
3ce0: c01dbe9c dec68110 df638940 df638940 ff8d dee83d2c dee83d08
3d00: c05e4ac8 c05e61f8 df638940 4000 df638800 df557000 0020 df638860
3d20: dee83d44 dee83d30 c05e4b50 c05e4a14 df638874 0400 dee83d54 dee83d48
3d40: c05e4ba0 c05e4af0 dee83d74 dee83d58 c034997c c05e4b90 df638840 df638800
3d60: dec3e4c0 dee83db4 dee83d78 c0368f70 c0349918 c03bfab0
3d80: df6388a4 df638afc dfff1bc2 df63898c df638988 df608800 0040 df701000
3da0: df63898c dee83e28 dee83e1c dee83db8 c0393514 c0368e88 df701000 df608800
3dc0: 0020 df638afc 030c 0188 df63898c df638800 0044 014000c0
3de0: df638ae8 0040 dee83e20 df701000 0040 dee83e80 c0392d48 df61ff00
3e00: df241200 dee83e80 0004a150 dee83e6c dee83e20 c0641d8c c0392d54
3e20: dee83ea0 c0101254
3e40: dee4f400 df61ff00 dee4f400
3e60: dee83efc dee83e70 c0242950 c0641cfc dee83e80 c07edac4 8013
3e80: 0040 dee83e98 0001 c0101254 0004a298 0040
3ea0: f7f27003 0055 b6f27000 df644000 00f6 c0108ea4 0004a150
3ec0: e000 c021636c dee83ee4 dee83ed8 c021636c c02162d8 dee83efc 00049148
3ee0: dee4f400 df61ff00 df557200 dee83fa4 dee83f00 c0243db8 c024285c
3f00: dee83f1c c0d05f40 c0d98a98 014080c0 c0d9ad5c 0001 e000
3f20: dee83f20 dee83f20 dee83f28 dee83f28 dee83f30 dee83f30
3f40: 0007 0004a298 0040
3f60: 0001 0006 011d b6f2bce8
3f80: 00f6 c0108ea4 dee82000 dee83fa8
3fa0: c0108ce0 c0243734 b6f2bce8 b6f27000 0001 0004a150 00049188
3fc0: b6f2bce8 00f6 0001 000490b8 000490d4
3fe0: bee3d838 bee3d828 b6ee63bc b6e73810 6010 b6f27000
[] (memcpy) from [] (atmel_aes_transfer_complete+0x64/0xe8)
[] (atmel_aes_transfer_complete) from []
(atmel_aes_ctr_transfer+0x174/0x194)
[] (atmel_aes_ctr_transfer) from []
(atmel_aes_cpu_transfer+0x17c/0x1b8)
[] (atmel_aes_cpu_transfer) from []
(atmel_aes_cpu_start+0xd0/0xd4)
[] (atmel_aes_cpu_start) from []
(atmel_aes_ctr_transfer+0x140/0x194)
[] (atmel_aes_ctr_transfer) from []
(atmel_aes_ctr_start+0x10c/0x15c)
[] (atmel_aes_ctr_start) from []
(atmel_aes_handle_queue+0xc0/0xdc)
[] (atmel_aes_handle_queue) from []
(atmel_aes_crypt+0x6c/0xa0)
[] (atmel_aes_crypt) from []
(atmel_aes_ctr_decrypt+0x1c/0x20)
[] (atmel_aes_ctr_decrypt) from []
(skcipher_decrypt_ablkcipher+0x70/0x74)
[] (skcipher_decrypt_ab
Re: [PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
2017-10-06 17:51 GMT+02:00 Romain Izard :
>
> Certain cipher modes like CTS expect the IV (req->info) of
> ablkcipher_request (or equivalently req->iv of skcipher_request) to
> contain the last ciphertext block when the {en,de}crypt operation is done.
>
> Fix this issue for the Atmel AES hardware engine. The tcrypt test
> case for cts(cbc(aes)) is now correctly passed.
>
> To handle the case of in-place decryption, copy the ciphertext in an
> intermediate buffer before decryption.
>
Unfortunately this does not seem to be enough. The tcrypt module's tests
pass, but I encounter more issues. If I run the libkcapi test suite, I
end up randomly with the following type of panic:
8< --
Unable to handle kernel paging request at virtual address 7ffc
pgd = dee9c000
[7ffc] *pgd=
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 2187 Comm: kcapi Not tainted 4.13.4+ #16
Hardware name: Atmel SAMA5
task: dec7f280 task.stack: dee82000
PC is at memcpy+0x114/0x330
LR is at atmel_aes_transfer_complete+0x64/0xe8
pc : []lr : []psr: 2013
sp : dee83bcc ip : 0003 fp : dee83bfc
r10: r9 : df638940 r8 : df638874
r7 : 0010 r6 : r5 : df638940 r4 : dec68110
r3 : 4004 r2 : 000c r1 : 7ffc r0 : df638afc
Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c53c7d Table: 3ee9c059 DAC: 0051
Process kcapi (pid: 2187, stack limit = 0xdee82208)
Stack: (0xdee83bcc to 0xdee84000)
3bc0:df638afc dec68110 c05e419c
3be0: 0030 dec68110 df557040 0030 dee83c3c dee83c00 c05e61cc c05e4144
3c00: 10031000 dec68110 df6388a4 0030 df6388a4 dec68110 df6388a4 0030
3c20: 0030 df638874 df557070 dee83c6c dee83c40 c05e488c c05e6064
3c40: df6388a4 df638874 dee83c6c dec68110 0030 0030 df6388a4 df638874
3c60: dee83c94 dee83c70 c05e4998 c05e471c 0030 dec68110 df557040
3c80: df638874 df557070 dee83cd4 dee83c98 c05e6198 c05e48d4 c05e6058
3ca0: dee83cbc df6388a4 c041ab04 dec68110 df557040 df638940 ff8d
3cc0: a013 0004 dee83d04 dee83cd8 c05e62f8 c05e6064 dee83d3c dee83ce8
3ce0: c01dbe9c dec68110 df638940 df638940 ff8d dee83d2c dee83d08
3d00: c05e4ac8 c05e61f8 df638940 4000 df638800 df557000 0020 df638860
3d20: dee83d44 dee83d30 c05e4b50 c05e4a14 df638874 0400 dee83d54 dee83d48
3d40: c05e4ba0 c05e4af0 dee83d74 dee83d58 c034997c c05e4b90 df638840 df638800
3d60: dec3e4c0 dee83db4 dee83d78 c0368f70 c0349918 c03bfab0
3d80: df6388a4 df638afc dfff1bc2 df63898c df638988 df608800 0040 df701000
3da0: df63898c dee83e28 dee83e1c dee83db8 c0393514 c0368e88 df701000 df608800
3dc0: 0020 df638afc 030c 0188 df63898c df638800 0044 014000c0
3de0: df638ae8 0040 dee83e20 df701000 0040 dee83e80 c0392d48 df61ff00
3e00: df241200 dee83e80 0004a150 dee83e6c dee83e20 c0641d8c c0392d54
3e20: dee83ea0 c0101254
3e40: dee4f400 df61ff00 dee4f400
3e60: dee83efc dee83e70 c0242950 c0641cfc dee83e80 c07edac4 8013
3e80: 0040 dee83e98 0001 c0101254 0004a298 0040
3ea0: f7f27003 0055 b6f27000 df644000 00f6 c0108ea4 0004a150
3ec0: e000 c021636c dee83ee4 dee83ed8 c021636c c02162d8 dee83efc 00049148
3ee0: dee4f400 df61ff00 df557200 dee83fa4 dee83f00 c0243db8 c024285c
3f00: dee83f1c c0d05f40 c0d98a98 014080c0 c0d9ad5c 0001 e000
3f20: dee83f20 dee83f20 dee83f28 dee83f28 dee83f30 dee83f30
3f40: 0007 0004a298 0040
3f60: 0001 0006 011d b6f2bce8
3f80: 00f6 c0108ea4 dee82000 dee83fa8
3fa0: c0108ce0 c0243734 b6f2bce8 b6f27000 0001 0004a150 00049188
3fc0: b6f2bce8 00f6 0001 000490b8 000490d4
3fe0: bee3d838 bee3d828 b6ee63bc b6e73810 6010 b6f27000
[] (memcpy) from [] (atmel_aes_transfer_complete+0x64/0xe8)
[] (atmel_aes_transfer_complete) from []
(atmel_aes_ctr_transfer+0x174/0x194)
[] (atmel_aes_ctr_transfer) from []
(atmel_aes_cpu_transfer+0x17c/0x1b8)
[] (atmel_aes_cpu_transfer) from []
(atmel_aes_cpu_start+0xd0/0xd4)
[] (atmel_aes_cpu_start) from []
(atmel_aes_ctr_transfer+0x140/0x194)
[] (atmel_aes_ctr_transfer) from []
(atmel_aes_ctr_start+0x10c/0x15c)
[] (atmel_aes_ctr_start) from []
(atmel_aes_handle_queue+0xc0/0xdc)
[] (atmel_aes_handle_queue) from []
(atmel_aes_crypt+0x6c/0xa0)
[] (atmel_aes_crypt) from []
(atmel_aes_ctr_decrypt+0x1c/0x20)
[] (atmel_aes_ctr_decrypt) from []
(skcipher_decrypt_ablkcipher+0x70/0x74)
[] (skcipher_decrypt_ablkcipher) from []
(crypto_ccm_decryp
[PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
Certain cipher modes like CTS expect the IV (req->info) of
ablkcipher_request (or equivalently req->iv of skcipher_request) to
contain the last ciphertext block when the {en,de}crypt operation is done.
Fix this issue for the Atmel AES hardware engine. The tcrypt test
case for cts(cbc(aes)) is now correctly passed.
To handle the case of in-place decryption, copy the ciphertext in an
intermediate buffer before decryption.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
drivers/crypto/atmel-aes.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 29e20c37f3a6..f22300babb45 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -156,6 +156,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
+ u8 *backup_info;
};
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
@@ -496,6 +497,12 @@ static void atmel_aes_authenc_complete(struct
atmel_aes_dev *dd, int err);
static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
{
+ struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq);
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
+ struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+ bool enc = atmel_aes_is_encrypt(dd);
+
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
atmel_aes_authenc_complete(dd, err);
#endif
@@ -503,6 +510,15 @@ static inline int atmel_aes_complete(struct atmel_aes_dev
*dd, int err)
clk_disable(dd->iclk);
dd->flags &= ~AES_FLAGS_BUSY;
+ if (enc) {
+ scatterwalk_map_and_copy(req->info, req->dst,
+req->nbytes - ivsize, ivsize, 0);
+ } else if (rctx->backup_info) {
+ memcpy(req->info, rctx->backup_info, ivsize);
+ kfree(rctx->backup_info);
+ rctx->backup_info = NULL;
+ }
+
if (dd->is_async)
dd->areq->complete(dd->areq, err);
@@ -959,13 +975,25 @@ static int atmel_aes_transfer_complete(struct
atmel_aes_dev *dd)
static int atmel_aes_start(struct atmel_aes_dev *dd)
{
struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq);
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+ bool enc = atmel_aes_is_encrypt(dd);
bool use_dma = (req->nbytes >= ATMEL_AES_DMA_THRESHOLD ||
dd->ctx->block_size != AES_BLOCK_SIZE);
int err;
atmel_aes_set_mode(dd, rctx);
+ if (!enc) {
+ rctx->backup_info = kzalloc(ivsize, GFP_KERNEL);
+ if (rctx->backup_info == NULL)
+ return atmel_aes_complete(dd, -ENOMEM);
+
+ scatterwalk_map_and_copy(rctx->backup_info, req->src,
+(req->nbytes - ivsize), ivsize, 0);
+ }
+
err = atmel_aes_hw_init(dd);
if (err)
return atmel_aes_complete(dd, err);
--
2.11.0
[PATCH] crypto: atmel-aes - properly set IV after {en,de}crypt
Certain cipher modes like CTS expect the IV (req->info) of
ablkcipher_request (or equivalently req->iv of skcipher_request) to
contain the last ciphertext block when the {en,de}crypt operation is done.
Fix this issue for the Atmel AES hardware engine. The tcrypt test
case for cts(cbc(aes)) is now correctly passed.
To handle the case of in-place decryption, copy the ciphertext in an
intermediate buffer before decryption.
Signed-off-by: Romain Izard
---
drivers/crypto/atmel-aes.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 29e20c37f3a6..f22300babb45 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -156,6 +156,7 @@ struct atmel_aes_authenc_ctx {
struct atmel_aes_reqctx {
unsigned long mode;
+ u8 *backup_info;
};
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
@@ -496,6 +497,12 @@ static void atmel_aes_authenc_complete(struct
atmel_aes_dev *dd, int err);
static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
{
+ struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq);
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
+ struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+ bool enc = atmel_aes_is_encrypt(dd);
+
#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
atmel_aes_authenc_complete(dd, err);
#endif
@@ -503,6 +510,15 @@ static inline int atmel_aes_complete(struct atmel_aes_dev
*dd, int err)
clk_disable(dd->iclk);
dd->flags &= ~AES_FLAGS_BUSY;
+ if (enc) {
+ scatterwalk_map_and_copy(req->info, req->dst,
+req->nbytes - ivsize, ivsize, 0);
+ } else if (rctx->backup_info) {
+ memcpy(req->info, rctx->backup_info, ivsize);
+ kfree(rctx->backup_info);
+ rctx->backup_info = NULL;
+ }
+
if (dd->is_async)
dd->areq->complete(dd->areq, err);
@@ -959,13 +975,25 @@ static int atmel_aes_transfer_complete(struct
atmel_aes_dev *dd)
static int atmel_aes_start(struct atmel_aes_dev *dd)
{
struct ablkcipher_request *req = ablkcipher_request_cast(dd->areq);
+ struct crypto_ablkcipher *ablkcipher = crypto_ablkcipher_reqtfm(req);
struct atmel_aes_reqctx *rctx = ablkcipher_request_ctx(req);
+ int ivsize = crypto_ablkcipher_ivsize(ablkcipher);
+ bool enc = atmel_aes_is_encrypt(dd);
bool use_dma = (req->nbytes >= ATMEL_AES_DMA_THRESHOLD ||
dd->ctx->block_size != AES_BLOCK_SIZE);
int err;
atmel_aes_set_mode(dd, rctx);
+ if (!enc) {
+ rctx->backup_info = kzalloc(ivsize, GFP_KERNEL);
+ if (rctx->backup_info == NULL)
+ return atmel_aes_complete(dd, -ENOMEM);
+
+ scatterwalk_map_and_copy(rctx->backup_info, req->src,
+(req->nbytes - ivsize), ivsize, 0);
+ }
+
err = atmel_aes_hw_init(dd);
if (err)
return atmel_aes_complete(dd, err);
--
2.11.0
[PATCH v4 2/8] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.11.0
[PATCH v4 2/8] clk: at91: pmc: Save SCSR during suspend
The contents of the System Clock Status Register (SCSR) needs to be
restored into the System Clock Enable Register (SCER).
As the bootloader will restore some clocks by itself, the issue can be
missed as only the USB controller, the LCD controller, the Image Sensor
controller and the programmable clocks will be impacted.
Fix the obvious typo in the suspend/resume code, as the IMR register
does not need to be saved twice.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
drivers/clk/at91/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 5c2b26de303e..07dc2861ad3f 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -86,7 +86,7 @@ static int pmc_suspend(void)
{
int i;
- regmap_read(pmcreg, AT91_PMC_IMR, _cache.scsr);
+ regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
regmap_read(pmcreg, AT91_CKGR_UCKR, _cache.uckr);
regmap_read(pmcreg, AT91_CKGR_MOR, _cache.mor);
@@ -129,7 +129,7 @@ static void pmc_resume(void)
if (pmc_cache.pllar != tmp)
pr_warn("PLLAR was not configured properly by the firmware\n");
- regmap_write(pmcreg, AT91_PMC_IMR, pmc_cache.scsr);
+ regmap_write(pmcreg, AT91_PMC_SCER, pmc_cache.scsr);
regmap_write(pmcreg, AT91_PMC_PCER, pmc_cache.pcsr0);
regmap_write(pmcreg, AT91_CKGR_UCKR, pmc_cache.uckr);
regmap_write(pmcreg, AT91_CKGR_MOR, pmc_cache.mor);
--
2.11.0
[PATCH v4 3/8] clk: at91: pmc: Support backup for programmable clocks
From: Romain Izard <romain.iz...@mobile-devices.fr>
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.11.0
[PATCH v4 4/8] mtd: nand: atmel: Avoid ECC errors when leaving backup mode
During backup mode, the contents of all registers will be cleared as the
SoC will be completely powered down. For a product that boots on NAND
Flash memory, the bootloader will obviously use the related controller
to read the Flash and correct any detected error in the memory, before
handling back control to the kernel's resuming entry point.
But it does not clean the NAND controller registers after use and on its
side the kernel driver expects the error locator to be powered down and
in a clean state. Add a resume hook for the PMECC error locator, and
reset its registers.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
---
Changes in v3:
* keep the PMECC disabled when not in use, and use atmel_pmecc_resume to
reset the controller after the bootloader has left it enabled.
Changes in v4:
* export atmel_pmecc_reset instead of atmel_pmecc_resume
* use the correct pointer in atmel_nand_controller_resume
drivers/mtd/nand/atmel/nand-controller.c | 3 +++
drivers/mtd/nand/atmel/pmecc.c | 17 +
drivers/mtd/nand/atmel/pmecc.h | 1 +
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/atmel/nand-controller.c
b/drivers/mtd/nand/atmel/nand-controller.c
index f25eca79f4e5..8afcff9a66ea 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -2530,6 +2530,9 @@ static __maybe_unused int
atmel_nand_controller_resume(struct device *dev)
struct atmel_nand_controller *nc = dev_get_drvdata(dev);
struct atmel_nand *nand;
+ if (nc->pmecc)
+ atmel_pmecc_reset(nc->pmecc);
+
list_for_each_entry(nand, >chips, node) {
int i;
diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c
index 146af8218314..0a3f12141c45 100644
--- a/drivers/mtd/nand/atmel/pmecc.c
+++ b/drivers/mtd/nand/atmel/pmecc.c
@@ -765,6 +765,13 @@ void atmel_pmecc_get_generated_eccbytes(struct
atmel_pmecc_user *user,
}
EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);
+void atmel_pmecc_reset(struct atmel_pmecc *pmecc)
+{
+ writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+}
+EXPORT_SYMBOL_GPL(atmel_pmecc_reset);
+
int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op)
{
struct atmel_pmecc *pmecc = user->pmecc;
@@ -797,10 +804,7 @@ EXPORT_SYMBOL_GPL(atmel_pmecc_enable);
void atmel_pmecc_disable(struct atmel_pmecc_user *user)
{
- struct atmel_pmecc *pmecc = user->pmecc;
-
- writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
- writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ atmel_pmecc_reset(user->pmecc);
mutex_unlock(>pmecc->lock);
}
EXPORT_SYMBOL_GPL(atmel_pmecc_disable);
@@ -855,10 +859,7 @@ static struct atmel_pmecc *atmel_pmecc_create(struct
platform_device *pdev,
/* Disable all interrupts before registering the PMECC handler. */
writel(0x, pmecc->regs.base + ATMEL_PMECC_IDR);
-
- /* Reset the ECC engine */
- writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
- writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ atmel_pmecc_reset(pmecc);
return pmecc;
}
diff --git a/drivers/mtd/nand/atmel/pmecc.h b/drivers/mtd/nand/atmel/pmecc.h
index a8ddbfca2ea5..817e0dd9fd15 100644
--- a/drivers/mtd/nand/atmel/pmecc.h
+++ b/drivers/mtd/nand/atmel/pmecc.h
@@ -61,6 +61,7 @@ atmel_pmecc_create_user(struct atmel_pmecc *pmecc,
struct atmel_pmecc_user_req *req);
void atmel_pmecc_destroy_user(struct atmel_pmecc_user *user);
+void atmel_pmecc_reset(struct atmel_pmecc *pmecc);
int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op);
void atmel_pmecc_disable(struct atmel_pmecc_user *user);
int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user);
--
2.11.0
[PATCH v4 3/8] clk: at91: pmc: Support backup for programmable clocks
From: Romain Izard
When an AT91 programmable clock is declared in the device tree, register
it into the Power Management Controller driver. On entering suspend mode,
the driver saves and restores the Programmable Clock registers to support
the backup mode for these clocks.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
---
Changes in v2:
* register PCKs on clock startup
Changes in v3:
* improve comments on hanling 0 in pmc_register_id and pmc_register_pck
* declare local variables earlier for checkpatch
drivers/clk/at91/clk-programmable.c | 2 ++
drivers/clk/at91/pmc.c | 35 +++
drivers/clk/at91/pmc.h | 2 ++
3 files changed, 39 insertions(+)
diff --git a/drivers/clk/at91/clk-programmable.c
b/drivers/clk/at91/clk-programmable.c
index 85a449cf61e3..0e6aab1252fc 100644
--- a/drivers/clk/at91/clk-programmable.c
+++ b/drivers/clk/at91/clk-programmable.c
@@ -204,6 +204,8 @@ at91_clk_register_programmable(struct regmap *regmap,
if (ret) {
kfree(prog);
hw = ERR_PTR(ret);
+ } else {
+ pmc_register_pck(id);
}
return hw;
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 07dc2861ad3f..1fa27f4ea538 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -22,6 +22,7 @@
#include "pmc.h"
#define PMC_MAX_IDS 128
+#define PMC_MAX_PCKS 8
int of_at91_get_clk_range(struct device_node *np, const char *propname,
struct clk_range *range)
@@ -50,6 +51,7 @@ EXPORT_SYMBOL_GPL(of_at91_get_clk_range);
static struct regmap *pmcreg;
static u8 registered_ids[PMC_MAX_IDS];
+static u8 registered_pcks[PMC_MAX_PCKS];
static struct
{
@@ -66,8 +68,13 @@ static struct
u32 pcr[PMC_MAX_IDS];
u32 audio_pll0;
u32 audio_pll1;
+ u32 pckr[PMC_MAX_PCKS];
} pmc_cache;
+/*
+ * As Peripheral ID 0 is invalid on AT91 chips, the identifier is stored
+ * without alteration in the table, and 0 is for unused clocks.
+ */
void pmc_register_id(u8 id)
{
int i;
@@ -82,9 +89,28 @@ void pmc_register_id(u8 id)
}
}
+/*
+ * As Programmable Clock 0 is valid on AT91 chips, there is an offset
+ * of 1 between the stored value and the real clock ID.
+ */
+void pmc_register_pck(u8 pck)
+{
+ int i;
+
+ for (i = 0; i < PMC_MAX_PCKS; i++) {
+ if (registered_pcks[i] == 0) {
+ registered_pcks[i] = pck + 1;
+ break;
+ }
+ if (registered_pcks[i] == (pck + 1))
+ break;
+ }
+}
+
static int pmc_suspend(void)
{
int i;
+ u8 num;
regmap_read(pmcreg, AT91_PMC_SCSR, _cache.scsr);
regmap_read(pmcreg, AT91_PMC_PCSR, _cache.pcsr0);
@@ -103,6 +129,10 @@ static int pmc_suspend(void)
regmap_read(pmcreg, AT91_PMC_PCR,
_cache.pcr[registered_ids[i]]);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_read(pmcreg, AT91_PMC_PCKR(num), _cache.pckr[num]);
+ }
return 0;
}
@@ -119,6 +149,7 @@ static bool pmc_ready(unsigned int mask)
static void pmc_resume(void)
{
int i;
+ u8 num;
u32 tmp;
u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
@@ -143,6 +174,10 @@ static void pmc_resume(void)
pmc_cache.pcr[registered_ids[i]] |
AT91_PMC_PCR_CMD);
}
+ for (i = 0; registered_pcks[i]; i++) {
+ num = registered_pcks[i] - 1;
+ regmap_write(pmcreg, AT91_PMC_PCKR(num), pmc_cache.pckr[num]);
+ }
if (pmc_cache.uckr & AT91_PMC_UPLLEN)
mask |= AT91_PMC_LOCKU;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 858e8ef7e8db..d22b1fa9ecdc 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -31,8 +31,10 @@ int of_at91_get_clk_range(struct device_node *np, const char
*propname,
#ifdef CONFIG_PM
void pmc_register_id(u8 id);
+void pmc_register_pck(u8 pck);
#else
static inline void pmc_register_id(u8 id) {}
+static inline void pmc_register_pck(u8 pck) {}
#endif
#endif /* __PMC_H_ */
--
2.11.0
[PATCH v4 4/8] mtd: nand: atmel: Avoid ECC errors when leaving backup mode
During backup mode, the contents of all registers will be cleared as the
SoC will be completely powered down. For a product that boots on NAND
Flash memory, the bootloader will obviously use the related controller
to read the Flash and correct any detected error in the memory, before
handling back control to the kernel's resuming entry point.
But it does not clean the NAND controller registers after use and on its
side the kernel driver expects the error locator to be powered down and
in a clean state. Add a resume hook for the PMECC error locator, and
reset its registers.
Signed-off-by: Romain Izard
---
Changes in v3:
* keep the PMECC disabled when not in use, and use atmel_pmecc_resume to
reset the controller after the bootloader has left it enabled.
Changes in v4:
* export atmel_pmecc_reset instead of atmel_pmecc_resume
* use the correct pointer in atmel_nand_controller_resume
drivers/mtd/nand/atmel/nand-controller.c | 3 +++
drivers/mtd/nand/atmel/pmecc.c | 17 +
drivers/mtd/nand/atmel/pmecc.h | 1 +
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/mtd/nand/atmel/nand-controller.c
b/drivers/mtd/nand/atmel/nand-controller.c
index f25eca79f4e5..8afcff9a66ea 100644
--- a/drivers/mtd/nand/atmel/nand-controller.c
+++ b/drivers/mtd/nand/atmel/nand-controller.c
@@ -2530,6 +2530,9 @@ static __maybe_unused int
atmel_nand_controller_resume(struct device *dev)
struct atmel_nand_controller *nc = dev_get_drvdata(dev);
struct atmel_nand *nand;
+ if (nc->pmecc)
+ atmel_pmecc_reset(nc->pmecc);
+
list_for_each_entry(nand, >chips, node) {
int i;
diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c
index 146af8218314..0a3f12141c45 100644
--- a/drivers/mtd/nand/atmel/pmecc.c
+++ b/drivers/mtd/nand/atmel/pmecc.c
@@ -765,6 +765,13 @@ void atmel_pmecc_get_generated_eccbytes(struct
atmel_pmecc_user *user,
}
EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);
+void atmel_pmecc_reset(struct atmel_pmecc *pmecc)
+{
+ writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+}
+EXPORT_SYMBOL_GPL(atmel_pmecc_reset);
+
int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op)
{
struct atmel_pmecc *pmecc = user->pmecc;
@@ -797,10 +804,7 @@ EXPORT_SYMBOL_GPL(atmel_pmecc_enable);
void atmel_pmecc_disable(struct atmel_pmecc_user *user)
{
- struct atmel_pmecc *pmecc = user->pmecc;
-
- writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
- writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ atmel_pmecc_reset(user->pmecc);
mutex_unlock(>pmecc->lock);
}
EXPORT_SYMBOL_GPL(atmel_pmecc_disable);
@@ -855,10 +859,7 @@ static struct atmel_pmecc *atmel_pmecc_create(struct
platform_device *pdev,
/* Disable all interrupts before registering the PMECC handler. */
writel(0x, pmecc->regs.base + ATMEL_PMECC_IDR);
-
- /* Reset the ECC engine */
- writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
- writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
+ atmel_pmecc_reset(pmecc);
return pmecc;
}
diff --git a/drivers/mtd/nand/atmel/pmecc.h b/drivers/mtd/nand/atmel/pmecc.h
index a8ddbfca2ea5..817e0dd9fd15 100644
--- a/drivers/mtd/nand/atmel/pmecc.h
+++ b/drivers/mtd/nand/atmel/pmecc.h
@@ -61,6 +61,7 @@ atmel_pmecc_create_user(struct atmel_pmecc *pmecc,
struct atmel_pmecc_user_req *req);
void atmel_pmecc_destroy_user(struct atmel_pmecc_user *user);
+void atmel_pmecc_reset(struct atmel_pmecc *pmecc);
int atmel_pmecc_enable(struct atmel_pmecc_user *user, int op);
void atmel_pmecc_disable(struct atmel_pmecc_user *user);
int atmel_pmecc_wait_rdy(struct atmel_pmecc_user *user);
--
2.11.0
[PATCH v4 0/8] Various patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the prototype version has been running the same test for more than a week without hanging. While all these patches are provided in a series, the clock, mtd, usb, pwm and mfd patch do not depend on each other. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Romain Izard (8): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks mtd: nand: atmel: Avoid ECC errors when leaving backup mode ehci-atmel: Power down during suspend is normal pwm: atmel-tcb: Support backup mode atmel_flexcom: Support backup mode tty/serial: atmel: Prevent a warning on suspend drivers/clk/at91/clk-programmable.c | 2 + drivers/clk/at91/pmc.c | 63 ++- drivers/clk/at91/pmc.h | 2 + drivers/mfd/atmel-flexcom.c | 65 drivers/mtd/nand/atmel/nand-controller.c | 3 ++ drivers/mtd/nand/atmel/pmecc.c | 17 + drivers/mtd/nand/atmel/pmecc.h | 1 + drivers/pwm/pwm-atmel-tcb.c | 63 ++- drivers/tty/serial/atmel_serial.c| 13 +++ drivers/usb/host/ehci-atmel.c| 3 +- 10 files changed, 196 insertions(+), 36 deletions(-) -- 2.11.0
[PATCH v4 0/8] Various patches for SAMA5D2 backup mode
While the core of the backup mode for SAMA5D2 has been integrated in v4.13, it is far from complete. Individual controllers in the chip have drivers that do not support the reset of the registers during suspend, and they need to be adapted to handle it. The first patch uses the clock wakeup code from the prototype backup mode instead of the version integrated in the mainline, as the mainline version is not stable. During a test loop with two-second backup suspend, the mainline version will hang in less than one day, whereas the prototype version has been running the same test for more than a week without hanging. While all these patches are provided in a series, the clock, mtd, usb, pwm and mfd patch do not depend on each other. Changes in v2: * drop the IIO patch duplicating existing code * determine the number of programmable clocks to save dynamically * declare a required local variable in the tty/serial patch Changes in v3: * drop dev_printk changes for PMECC * rework the resume code for PMECC * improve comments on PMC clock handling Changes in v4: * fix a bug in the PMECC resume code Romain Izard (8): clk: at91: pmc: Wait for clocks when resuming clk: at91: pmc: Save SCSR during suspend clk: at91: pmc: Support backup for programmable clocks mtd: nand: atmel: Avoid ECC errors when leaving backup mode ehci-atmel: Power down during suspend is normal pwm: atmel-tcb: Support backup mode atmel_flexcom: Support backup mode tty/serial: atmel: Prevent a warning on suspend drivers/clk/at91/clk-programmable.c | 2 + drivers/clk/at91/pmc.c | 63 ++- drivers/clk/at91/pmc.h | 2 + drivers/mfd/atmel-flexcom.c | 65 drivers/mtd/nand/atmel/nand-controller.c | 3 ++ drivers/mtd/nand/atmel/pmecc.c | 17 + drivers/mtd/nand/atmel/pmecc.h | 1 + drivers/pwm/pwm-atmel-tcb.c | 63 ++- drivers/tty/serial/atmel_serial.c| 13 +++ drivers/usb/host/ehci-atmel.c| 3 +- 10 files changed, 196 insertions(+), 36 deletions(-) -- 2.11.0
[PATCH v4 5/8] ehci-atmel: Power down during suspend is normal
When an Atmel SoC is suspended with the backup mode, the USB bus will be powered down. As this is expected, do not return an error to the driver core when ehci_resume detects it. Signed-off-by: Romain Izard <romain.izard@gmail.com> Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com> --- drivers/usb/host/ehci-atmel.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/ehci-atmel.c b/drivers/usb/host/ehci-atmel.c index 7440722bfbf0..2a8b9bdc0e57 100644 --- a/drivers/usb/host/ehci-atmel.c +++ b/drivers/usb/host/ehci-atmel.c @@ -205,7 +205,8 @@ static int __maybe_unused ehci_atmel_drv_resume(struct device *dev) struct atmel_ehci_priv *atmel_ehci = hcd_to_atmel_ehci_priv(hcd); atmel_start_clock(atmel_ehci); - return ehci_resume(hcd, false); + ehci_resume(hcd, false); + return 0; } #ifdef CONFIG_OF -- 2.11.0
[PATCH v4 1/8] clk: at91: pmc: Wait for clocks when resuming
Wait for the syncronization of all clocks when resuming, not only the
UPLL clock. Do not use regmap_read_poll_timeout, as it will call BUG()
when interrupts are masked, which is the case in here.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Ludovic Desroches <ludovic.desroc...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/clk/at91/pmc.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 775af473fe11..5c2b26de303e 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -107,10 +107,20 @@ static int pmc_suspend(void)
return 0;
}
+static bool pmc_ready(unsigned int mask)
+{
+ unsigned int status;
+
+ regmap_read(pmcreg, AT91_PMC_SR, );
+
+ return ((status & mask) == mask) ? 1 : 0;
+}
+
static void pmc_resume(void)
{
- int i, ret = 0;
+ int i;
u32 tmp;
+ u32 mask = AT91_PMC_MCKRDY | AT91_PMC_LOCKA;
regmap_read(pmcreg, AT91_PMC_MCKR, );
if (pmc_cache.mckr != tmp)
@@ -134,13 +144,11 @@ static void pmc_resume(void)
AT91_PMC_PCR_CMD);
}
- if (pmc_cache.uckr & AT91_PMC_UPLLEN) {
- ret = regmap_read_poll_timeout(pmcreg, AT91_PMC_SR, tmp,
- !(tmp & AT91_PMC_LOCKU),
- 10, 5000);
- if (ret)
- pr_crit("USB PLL didn't lock when resuming\n");
- }
+ if (pmc_cache.uckr & AT91_PMC_UPLLEN)
+ mask |= AT91_PMC_LOCKU;
+
+ while (!pmc_ready(mask))
+ cpu_relax();
}
static struct syscore_ops pmc_syscore_ops = {
--
2.11.0
[PATCH v4 8/8] tty/serial: atmel: Prevent a warning on suspend
The atmel serial port driver reported the following warning on suspend:
atmel_usart f802.serial: ttyS1: Unable to drain transmitter
As the ATMEL_US_TXEMPTY status bit in ATMEL_US_CSR is always cleared
when the transmitter is disabled, we need to know the transmitter's
state to return the real fifo state. And as ATMEL_US_CR is write-only,
it is necessary to save the state of the transmitter in a local
variable, and update the variable when TXEN and TXDIS is written in
ATMEL_US_CR.
After those changes, atmel_tx_empty can return "empty" on suspend, the
warning in uart_suspend_port disappears, and suspending is 20ms shorter
for each enabled Atmel serial port.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Tested-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Acked-by: Richard Genoud <richard.gen...@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/tty/serial/atmel_serial.c
b/drivers/tty/serial/atmel_serial.c
index 7551cab438ff..ce45b4ada0bf 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -171,6 +171,7 @@ struct atmel_uart_port {
boolhas_hw_timer;
struct timer_list uart_timer;
+ booltx_stopped;
boolsuspended;
unsigned intpending;
unsigned intpending_status;
@@ -380,6 +381,10 @@ static int atmel_config_rs485(struct uart_port *port,
*/
static u_int atmel_tx_empty(struct uart_port *port)
{
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+
+ if (atmel_port->tx_stopped)
+ return TIOCSER_TEMT;
return (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXEMPTY) ?
TIOCSER_TEMT :
0;
@@ -485,6 +490,7 @@ static void atmel_stop_tx(struct uart_port *port)
* is fully transmitted.
*/
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
+ atmel_port->tx_stopped = true;
/* Disable interrupts */
atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
@@ -521,6 +527,7 @@ static void atmel_start_tx(struct uart_port *port)
/* re-enable the transmitter */
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+ atmel_port->tx_stopped = false;
}
/*
@@ -1866,6 +1873,7 @@ static int atmel_startup(struct uart_port *port)
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
/* enable xmit & rcvr */
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+ atmel_port->tx_stopped = false;
setup_timer(_port->uart_timer,
atmel_uart_timer_callback,
@@ -2122,6 +2130,7 @@ static void atmel_set_termios(struct uart_port *port,
struct ktermios *termios,
/* disable receiver and transmitter */
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS | ATMEL_US_RXDIS);
+ atmel_port->tx_stopped = true;
/* mode */
if (port->rs485.flags & SER_RS485_ENABLED) {
@@ -2207,6 +2216,7 @@ static void atmel_set_termios(struct uart_port *port,
struct ktermios *termios,
atmel_uart_writel(port, ATMEL_US_BRGR, quot);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+ atmel_port->tx_stopped = false;
/* restore interrupts */
atmel_uart_writel(port, ATMEL_US_IER, imr);
@@ -2450,6 +2460,7 @@ static void atmel_console_write(struct console *co, const
char *s, u_int count)
/* Make sure that tx path is actually able to send characters */
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+ atmel_port->tx_stopped = false;
uart_console_write(port, s, count, atmel_console_putchar);
@@ -2511,6 +2522,7 @@ static int __init atmel_console_setup(struct console *co,
char *options)
{
int ret;
struct uart_port *port = _ports[co->index].uart;
+ struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
int baud = 115200;
int bits = 8;
int parity = 'n';
@@ -2528,6 +2540,7 @@ static int __init atmel_console_setup(struct console *co,
char *options)
atmel_uart_writel(port, ATMEL_US_IDR, -1);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_RSTSTA | ATMEL_US_RSTRX);
atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN | ATMEL_US_RXEN);
+ atmel_port->tx_stopped = false;
if (options)
uart_parse_options(options, , , , );
--
2.11.0
[PATCH v4 7/8] atmel_flexcom: Support backup mode
The controller used by a flexcom module is configured at boot, and left
alone after this. As the configuration will be lost after backup mode,
restore the state of the flexcom driver on resume.
Signed-off-by: Romain Izard <romain.izard@gmail.com>
Acked-by: Nicolas Ferre <nicolas.fe...@microchip.com>
Tested-by: Nicolas Ferre <nicolas.fe...@microchip.com>
---
drivers/mfd/atmel-flexcom.c | 65 ++---
1 file changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..ef1235c4a179 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,44 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *afc;
int err;
+ u32 val;
+
+ afc = devm_kzalloc(>dev, sizeof(*afc), GFP_KERNEL);
+ if (!afc)
+ return -ENOMEM;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ platform_set_drvdata(pdev, afc);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (afc->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ afc->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ afc->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(afc->base))
+ return PTR_ERR(afc->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ afc->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(afc->clk))
+ return PTR_ERR(afc->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(afc->clk);
if (err)
return err;
@@ -76,9 +86,10 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ val = FLEX_MR_OPMODE(afc->opmode);
+ writel(val, afc->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(afc->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +100,34 @@ static const struct of_device_id atmel_flexcom_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *afc = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(afc->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(afc->opmode),
+ writel(val, afc->base + FLEX_MR);
+
+ clk_disable_unprepare(afc->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.11.0
[PATCH v4 7/8] atmel_flexcom: Support backup mode
The controller used by a flexcom module is configured at boot, and left
alone after this. As the configuration will be lost after backup mode,
restore the state of the flexcom driver on resume.
Signed-off-by: Romain Izard
Acked-by: Nicolas Ferre
Tested-by: Nicolas Ferre
---
drivers/mfd/atmel-flexcom.c | 65 ++---
1 file changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/mfd/atmel-flexcom.c b/drivers/mfd/atmel-flexcom.c
index 064bde9cff5a..ef1235c4a179 100644
--- a/drivers/mfd/atmel-flexcom.c
+++ b/drivers/mfd/atmel-flexcom.c
@@ -39,34 +39,44 @@
#define FLEX_MR_OPMODE(opmode) (((opmode) << FLEX_MR_OPMODE_OFFSET) & \
FLEX_MR_OPMODE_MASK)
+struct atmel_flexcom {
+ void __iomem *base;
+ u32 opmode;
+ struct clk *clk;
+};
static int atmel_flexcom_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct clk *clk;
struct resource *res;
- void __iomem *base;
- u32 opmode;
+ struct atmel_flexcom *afc;
int err;
+ u32 val;
+
+ afc = devm_kzalloc(>dev, sizeof(*afc), GFP_KERNEL);
+ if (!afc)
+ return -ENOMEM;
- err = of_property_read_u32(np, "atmel,flexcom-mode", );
+ platform_set_drvdata(pdev, afc);
+
+ err = of_property_read_u32(np, "atmel,flexcom-mode", >opmode);
if (err)
return err;
- if (opmode < ATMEL_FLEXCOM_MODE_USART ||
- opmode > ATMEL_FLEXCOM_MODE_TWI)
+ if (afc->opmode < ATMEL_FLEXCOM_MODE_USART ||
+ afc->opmode > ATMEL_FLEXCOM_MODE_TWI)
return -EINVAL;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(>dev, res);
- if (IS_ERR(base))
- return PTR_ERR(base);
+ afc->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(afc->base))
+ return PTR_ERR(afc->base);
- clk = devm_clk_get(>dev, NULL);
- if (IS_ERR(clk))
- return PTR_ERR(clk);
+ afc->clk = devm_clk_get(>dev, NULL);
+ if (IS_ERR(afc->clk))
+ return PTR_ERR(afc->clk);
- err = clk_prepare_enable(clk);
+ err = clk_prepare_enable(afc->clk);
if (err)
return err;
@@ -76,9 +86,10 @@ static int atmel_flexcom_probe(struct platform_device *pdev)
* inaccessible and are read as zero. Also the external I/O lines of the
* Flexcom are muxed to reach the selected device.
*/
- writel(FLEX_MR_OPMODE(opmode), base + FLEX_MR);
+ val = FLEX_MR_OPMODE(afc->opmode);
+ writel(val, afc->base + FLEX_MR);
- clk_disable_unprepare(clk);
+ clk_disable_unprepare(afc->clk);
return devm_of_platform_populate(>dev);
}
@@ -89,10 +100,34 @@ static const struct of_device_id atmel_flexcom_of_match[]
= {
};
MODULE_DEVICE_TABLE(of, atmel_flexcom_of_match);
+#ifdef CONFIG_PM_SLEEP
+static int atmel_flexcom_resume(struct device *dev)
+{
+ struct atmel_flexcom *afc = dev_get_drvdata(dev);
+ int err;
+ u32 val;
+
+ err = clk_prepare_enable(afc->clk);
+ if (err)
+ return err;
+
+ val = FLEX_MR_OPMODE(afc->opmode),
+ writel(val, afc->base + FLEX_MR);
+
+ clk_disable_unprepare(afc->clk);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_flexcom_pm_ops, NULL,
+atmel_flexcom_resume);
+
static struct platform_driver atmel_flexcom_driver = {
.probe = atmel_flexcom_probe,
.driver = {
.name = "atmel_flexcom",
+ .pm = _flexcom_pm_ops,
.of_match_table = atmel_flexcom_of_match,
},
};
--
2.11.0
