Re: [RFC PATCH v3 0/7] Add virtio_rtc module and related changes

2024-03-13 Thread Alexandre Belloni
On 13/03/2024 14:06:42+, David Woodhouse wrote:
> If you're asking why patch 7/7 in Peter's series exists to expose the
> virtio clock through RTC, and you're not particularly interested in the
> first six, I suppose that's a fair question. As is the question of "why
> is it called virtio_rtc not virtio_ptp?". 
> 

Exactly my question, thanks :)

> But let me turn it around: if the kernel has access to this virtio
> device and *not* any other RTC, why *wouldn't* the kernel use the time
> from it? The fact that it can optionally *also* provide paired readings
> with the CPU counter doesn't actually *hurt* for the RTC use case, does
> it?

As long as it doesn't behave differently from the other RTC, I'm fine
with this. This is important because I don't want to carry any special
infrastructure for this driver or to have to special case this driver
later on because it is incompatible with some evolution of the
subsystem.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



Re: [RFC PATCH v3 0/7] Add virtio_rtc module and related changes

2024-03-13 Thread Alexandre Belloni
On 13/03/2024 12:29:38+, David Woodhouse wrote:
> On Wed, 2024-03-13 at 12:18 +0100, Alexandre Belloni wrote:
> > 
> > I still don't know anything about virtio but under Linux, an RTC is
> > always UTC (or localtime when dual booting but let's not care) and never
> > accounts for leap seconds. Having an RTC and RTC driver behaving
> > differently would be super inconvenient. Why don't you leave this to
> > userspace?
> 
> Well yes, we don't need to expose *anything* from the hypervisor and we
> can leave it all to guest userspace. We can run NTP on every single one
> of *hundreds* of guests, leaving them all to duplicate the work of
> calibrating the *same* underlying oscillator.
> 

Really, I see the point of sharing the time accurately between the host
and the guest and not duplicating the effort. This is not what I am
objecting to.

> I thought we were trying to avoid that, by having the hypervisor tell
> them what the time was. If we're going to do that, we need it to be
> sufficiently precise (and some clients want to *know* the precision),
> and above all we need it to be *unambiguous*.
> 
> If the hypervisor says that the time is 3692217600.001, then the guest
> doesn't actually know *which* 3692217600.001 it is, and thus it still
> doesn't know the time to an accuracy better than 1 second.
> 

The RTC subsystem has a 1 second resolution and this is not going to
change because there is no point doing so for the hardware, to get the
time precisely, UIE MUST be used there is no vendor that will just
support reading the time/date or timestamp as this is way too imprecise.

> And if we start allowing the hypervisor to smear clocks in some other
> underspecified ways, then we end up with errors of up to 1 second in
> the clock for long periods of time *around* the leap second.
> 
> We need to avoid that ambiguity.

Exactly my point and I said, reading an RTC is always UTC and never
handles leap seconds so if userspace doesn't handle the leap second and
updates the RTC, too bad. There are obviously no RTC that will smear
clock unless instructed to, so the hypervisor must not smear the clock.

Note that this is not an issue for the subsystem because if you are not
capable to track an accurate clock, the RTC itself will have drifted by
much more than a second in between leap second inclusions.

> 
> > I guess I'm still questioning whether this is the correct interface to
> > expose the host system time instead of an actual RTC.
> 
> If an RTC device is able to report '23:59:60' as the time of day, I
> suppose that *could* resolve the ambiguity. But talking to a device is
> slow; we want guests to be able to know the time — accurately — with a
> simple counter/tsc read and some arithmetic. Which means *paired* reads
> of 'RTC' and the counter, and a precise indication of the counter
> frequency.

23:59:60 is not and will never be allowed in the RTC subsystem as this
is an invalid value for the hardware.

The TSC or whatever CPU counter/clock that is used to keep the system
time is not an RTC, I don't get why it has to be exposed as such to the
guests. PTP is fine and precise, RTC is not.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



Re: [RFC PATCH v3 0/7] Add virtio_rtc module and related changes

2024-03-13 Thread Alexandre Belloni
On 13/03/2024 10:45:54+0100, Peter Hilber wrote:
> > Exposing UTC as the only clock reference is bad enough; when leap
> > seconds happen there's a whole second during which you don't *know*
> > which second it is. It seems odd to me, for a precision clock to be
> > deliberately ambiguous about what the time is!
> 
> Just to be clear, the device can perfectly expose only a TAI reference
> clock (or both UTC and TAI), the spec is just completely open about this,
> as it tries to work for diverse use cases.
> 
> > 
> > But if the virtio-rtc clock is defined as UTC and then expose something
> > *different* in it, that's even worse. You potentially end up providing
> > inaccurate time for a whole *day* leading up to the leap second.
> > 
> > I think you're right that leap second smearing should be addressed. At
> > the very least, by making it clear that the virtio-rtc clock which
> > advertises UTC shall be used *only* for UTC, never UTC-SLS or any other
> > yet-to-be-defined variant.
> > 
> 
> Agreed.
> 
> > Please make it explicit that any hypervisor which wants to advertise a
> > smeared clock shall define a new type which specifies the precise
> > smearing algorithm and cannot be conflated with the one you're defining
> > here.
> > 
> 
> I will add a requirement that the UTC clock can never have smeared/smoothed
> leap seconds.
> 
> I think that not every vendor would bother to first add a definition of a
> smearing algorithm. Also, I think in some cases knowing the precise
> smearing algorithm might not be important (when having the same time as the
> hypervisor is enough and accuracy w.r.t. actual time is less important).
> 
> So maybe I should add a VIRTIO_RTC_CLOCK_UTC_SMEARED clock type, which for
> now could catch every UTC-like clock which smears/smoothes leap seconds,
> where the vendor cannot be bothered to add the smearing algorithm to spec
> and implementations.
> 

I still don't know anything about virtio but under Linux, an RTC is
always UTC (or localtime when dual booting but let's not care) and never
accounts for leap seconds. Having an RTC and RTC driver behaving
differently would be super inconvenient. Why don't you leave this to
userspace?

I guess I'm still questioning whether this is the correct interface to
expose the host system time instead of an actual RTC.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



Re: [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver

2024-03-11 Thread Alexandre Belloni
On 11/03/2024 19:28:50+0100, Peter Hilber wrote:
> >> Perhaps this might be handled by the driver also setting a virtio-rtc
> >> monotonic alarm (which uses a clock similar to CLOCK_BOOTTIME_ALARM).
> >> The
> >> virtio-rtc monotonic alarm would just be used to wake up in case it was
> >> a
> >> CLOCK_BOOTTIME_ALARM alarm.
> >> 
> >> Otherwise, the behavior should not differ from other RTC class drivers.
> >> 
> > 
> > What I don't quite get is how this is actually related to RTCs. This
> > would be a super imprecise mechanism to get the current time and date
> > from the host to the guest which is what I think your are trying to do,
> > especially since this is not supporting UIE.
> > The host system clock may come from reading the RTC at some point in
> > time but more likely from another source so is it really the best
> > synchronization mechanism?
> 
> Hello,
> 
> thank you for your comments.
> 
> The main motivation to have the RTC class driver is the RTC alarm
> (discussed below).
> 
> As for synchronization, virtio_rtc also offers a PTP clock [1] which will
> be more precise, but which needs a user space daemon. As for RTC-based
> initial synchronization, my idea was to propose, in a second step, an
> optional op for rtc_class_ops, which would read the clock with nanosecond
> precision. This optional op could then be used in rtc_hctosys(), so there
> would be no need for UIE waiting.

This would be a clear NAK, rtc_hctosys should use UIE to have proper
synchronisation. It currently does a very bad job reading the RTC and it
is a pity it has been mandated by systemd as useerspace is definitively
better placed to set the system time. I'm still very tempted delaying
everyone's boot by one second and make rtc_hctosys precise for all the
supported HW and not just a single driver.

> [1] 
> https://lore.kernel.org/all/20231218073849.35294-6-peter.hil...@opensynergy.com/
> 
> > 
> > The other thing is that I don't quite get the point of the RTC alarm
> > versus a regular timer in this context.
> 
> RTC alarms allow to resume from suspend and poweroff (esp. also through
> alarmtimers), which is of interest in embedded virtualization. In my
> understanding RTC is ATM the only way to do this.
> 
> (I was indeed thinking about adding an alternate alarmtimer backend for
> CLOCK_BOOTTIME_ALARM, which should deal with the CLOCK_REALTIME_ALARM vs
> CLOCK_BOOTTIME_ALARM issue which is described in the commit message.)
> 

Right but this seems like a super convoluted way of getting the host to
wakeup the guest...

> > 
> > 
> > [...]
> > 
> >> +static const struct rtc_class_ops viortc_class_with_alarm_ops = {
> >> +  .read_time = viortc_class_read_time,
> >> +  .read_alarm = viortc_class_read_alarm,
> >> +  .set_alarm = viortc_class_set_alarm,
> >> +  .alarm_irq_enable = viortc_class_alarm_irq_enable,
> >> +};
> >> +
> >> +static const struct rtc_class_ops viortc_class_no_alarm_ops = {
> >> +  .read_time = viortc_class_read_time,
> >> +};
> >> +
> > 
> > [...]
> > 
> >> +/**
> >> +/**
> >> + * viortc_class_init() - init RTC class wrapper and device
> >> + * @viortc: device data
> >> + * @vio_clk_id: virtio_rtc clock id
> >> + * @have_alarm: expose alarm ops
> >> + * @parent_dev: virtio device
> >> + *
> >> + * Context: Process context.
> >> + * Return: RTC class wrapper on success, ERR_PTR otherwise.
> >> + */
> >> +struct viortc_class *viortc_class_init(struct viortc_dev *viortc,
> >> + u16 vio_clk_id, bool have_alarm,
> >> + struct device *parent_dev)
> >> +{
> >> +  struct viortc_class *viortc_class;
> >> +  struct rtc_device *rtc;
> >> +
> >> +  viortc_class =
> >> +  devm_kzalloc(parent_dev, sizeof(*viortc_class),
> >> GFP_KERNEL);
> >> +  if (!viortc_class)
> >> +  return ERR_PTR(-ENOMEM);
> >> +
> >> +  viortc_class->viortc = viortc;
> >> +
> >> +  rtc = devm_rtc_allocate_device(parent_dev);
> >> +  if (IS_ERR(rtc))
> >> +  return ERR_PTR(PTR_ERR(rtc));
> >> +
> >> +  viortc_class->rtc = rtc;
> >> +
> >> +  clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> >> +
> >> +  rtc->ops = have_alarm ? _class_with_alarm_ops :
> >> +  _class_no_alarm_ops;
> > 
> > Don't do this, simply clear the alarm feature.
> > 
> 
> OK (sorry, was obviously very inelegant).
> 
> Best regards,
> 
> Peter

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



Re: [RFC PATCH v3 7/7] virtio_rtc: Add RTC class driver

2024-03-08 Thread Alexandre Belloni
Hello,

I'll start by saying that I'm sorry, I have a very very high level
knowledge about what virtio is.

On 18/12/2023 08:38:45+0100, Peter Hilber wrote:
> Expose the virtio-rtc UTC clock as an RTC clock to userspace, if it is
> present. Support RTC alarm if the virtio-rtc alarm feature is present. The
> virtio-rtc device signals an alarm by marking an alarmq buffer as used.
> 
> Peculiarities
> -
> 
> A virtio-rtc clock is a bit special for an RTC clock in that
> 
> - the clock may step (also backwards) autonomously at any time and
> 
> - the device, and its notification mechanism, will be reset during boot or
>   resume from sleep.
> 
> The virtio-rtc device avoids that the driver might miss an alarm. The
> device signals an alarm whenever the clock has reached or passed the alarm
> time, and also when the device is reset (on boot or resume from sleep), if
> the alarm time is in the past.
> 
> Open Issue
> --
> 
> The CLOCK_BOOTTIME_ALARM will use the RTC clock to wake up from sleep, and
> implicitly assumes that no RTC clock steps will occur during sleep. The RTC
> class driver does not know whether the current alarm is a real-time alarm
> or a boot-time alarm.
> 
> Perhaps this might be handled by the driver also setting a virtio-rtc
> monotonic alarm (which uses a clock similar to CLOCK_BOOTTIME_ALARM). The
> virtio-rtc monotonic alarm would just be used to wake up in case it was a
> CLOCK_BOOTTIME_ALARM alarm.
> 
> Otherwise, the behavior should not differ from other RTC class drivers.
> 

What I don't quite get is how this is actually related to RTCs. This
would be a super imprecise mechanism to get the current time and date
from the host to the guest which is what I think your are trying to do,
especially since this is not supporting UIE.
The host system clock may come from reading the RTC at some point in
time but more likely from another source so is it really the best
synchronization mechanism?

The other thing is that I don't quite get the point of the RTC alarm
versus a regular timer in this context.


[...]

> +static const struct rtc_class_ops viortc_class_with_alarm_ops = {
> + .read_time = viortc_class_read_time,
> + .read_alarm = viortc_class_read_alarm,
> + .set_alarm = viortc_class_set_alarm,
> + .alarm_irq_enable = viortc_class_alarm_irq_enable,
> +};
> +
> +static const struct rtc_class_ops viortc_class_no_alarm_ops = {
> + .read_time = viortc_class_read_time,
> +};
> +

[...]

> +/**
> +/**
> + * viortc_class_init() - init RTC class wrapper and device
> + * @viortc: device data
> + * @vio_clk_id: virtio_rtc clock id
> + * @have_alarm: expose alarm ops
> + * @parent_dev: virtio device
> + *
> + * Context: Process context.
> + * Return: RTC class wrapper on success, ERR_PTR otherwise.
> + */
> +struct viortc_class *viortc_class_init(struct viortc_dev *viortc,
> +u16 vio_clk_id, bool have_alarm,
> +struct device *parent_dev)
> +{
> + struct viortc_class *viortc_class;
> + struct rtc_device *rtc;
> +
> + viortc_class =
> + devm_kzalloc(parent_dev, sizeof(*viortc_class), GFP_KERNEL);
> + if (!viortc_class)
> + return ERR_PTR(-ENOMEM);
> +
> + viortc_class->viortc = viortc;
> +
> + rtc = devm_rtc_allocate_device(parent_dev);
> + if (IS_ERR(rtc))
> + return ERR_PTR(PTR_ERR(rtc));
> +
> + viortc_class->rtc = rtc;
> +
> + clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, rtc->features);
> +
> + rtc->ops = have_alarm ? _class_with_alarm_ops :
> + _class_no_alarm_ops;

Don't do this, simply clear the alarm feature.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



Re: [PATCH 1/1] rtc: pcf85063: add integrity check

2021-04-19 Thread Alexandre Belloni
On 19/04/2021 20:26:42+, Gervais, Francois wrote:
> > I'm not sure I get the use case because PCF85063_REG_CTRL2 should be
> > initialized properly after the driver is probed anyway. The other two
> > can be set from userspace once it detects the oscillator failure which
> > would be better at deciding the policy anyway.
> 
> Thank you for the feedback I think I understand now.
> 
> We saw the reported problem on devices running kernel v5.4 which doesn't
> have the common clock framework support and so PCF85063_REG_CTRL2
> clkout was not initialized by the kernel and left at hardware default.
> 
> I guess with CCF support, if PCF85063_REG_CTRL2 gets corrupted on
> power application, on driver probe the clkout value will be set to 0b000
> or some known default.
> 
> I'm not familiar the CCF, do you know if it's the case that default values
> will be set on boot?

The CCF will disable any clocks that are not used on boot so the clock
will be either used and configured or not used and disabled.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[PATCH 2/3] rtc: pcf8523: add alarm support

2021-04-17 Thread Alexandre Belloni
Alarm support requires unconditionally disabling clock out because it is
using the int1 pin.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-pcf8523.c | 181 +-
 1 file changed, 180 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c
index fe3ab41d8326..feadab8e3bd3 100644
--- a/drivers/rtc/rtc-pcf8523.c
+++ b/drivers/rtc/rtc-pcf8523.c
@@ -8,10 +8,15 @@
 #include 
 #include 
 #include 
+#include 
 
 #define REG_CONTROL1 0x00
 #define REG_CONTROL1_CAP_SEL BIT(7)
 #define REG_CONTROL1_STOPBIT(5)
+#define REG_CONTROL1_AIEBIT(1)
+
+#define REG_CONTROL2 0x01
+#define REG_CONTROL2_AF BIT(3)
 
 #define REG_CONTROL3 0x02
 #define REG_CONTROL3_PM_BLD BIT(7) /* battery low detection disabled */
@@ -30,9 +35,22 @@
 #define REG_MONTHS   0x08
 #define REG_YEARS0x09
 
+#define REG_MINUTE_ALARM   0x0a
+#define REG_HOUR_ALARM 0x0b
+#define REG_DAY_ALARM  0x0c
+#define REG_WEEKDAY_ALARM  0x0d
+#define ALARM_DIS BIT(7)
+
 #define REG_OFFSET   0x0e
 #define REG_OFFSET_MODE BIT(7)
 
+#define REG_TMR_CLKOUT_CTRL 0x0f
+
+struct pcf8523 {
+   struct rtc_device *rtc;
+   struct i2c_client *client;
+};
+
 static int pcf8523_read(struct i2c_client *client, u8 reg, u8 *valuep)
 {
struct i2c_msg msgs[2];
@@ -138,6 +156,27 @@ static int pcf8523_set_pm(struct i2c_client *client, u8 pm)
return 0;
 }
 
+static irqreturn_t pcf8523_irq(int irq, void *dev_id)
+{
+   struct pcf8523 *pcf8523 = i2c_get_clientdata(dev_id);
+   u8 value;
+   int err;
+
+   err = pcf8523_read(pcf8523->client, REG_CONTROL2, );
+   if (err < 0)
+   return IRQ_HANDLED;
+
+   if (value & REG_CONTROL2_AF) {
+   value &= ~REG_CONTROL2_AF;
+   pcf8523_write(pcf8523->client, REG_CONTROL2, value);
+   rtc_update_irq(pcf8523->rtc, 1, RTC_IRQF | RTC_AF);
+
+   return IRQ_HANDLED;
+   }
+
+   return IRQ_NONE;
+}
+
 static int pcf8523_stop_rtc(struct i2c_client *client)
 {
u8 value;
@@ -257,6 +296,111 @@ static int pcf8523_rtc_set_time(struct device *dev, 
struct rtc_time *tm)
return pcf8523_start_rtc(client);
 }
 
+static int pcf8523_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   u8 start = REG_MINUTE_ALARM, regs[4];
+   struct i2c_msg msgs[2];
+   u8 value;
+   int err;
+
+   msgs[0].addr = client->addr;
+   msgs[0].flags = 0;
+   msgs[0].len = 1;
+   msgs[0].buf = 
+
+   msgs[1].addr = client->addr;
+   msgs[1].flags = I2C_M_RD;
+   msgs[1].len = sizeof(regs);
+   msgs[1].buf = regs;
+
+   err = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (err < 0)
+   return err;
+
+   tm->time.tm_sec = 0;
+   tm->time.tm_min = bcd2bin(regs[0] & 0x7F);
+   tm->time.tm_hour = bcd2bin(regs[1] & 0x3F);
+   tm->time.tm_mday = bcd2bin(regs[2] & 0x3F);
+   tm->time.tm_wday = bcd2bin(regs[3] & 0x7);
+
+   err = pcf8523_read(client, REG_CONTROL1, );
+   if (err < 0)
+   return err;
+   tm->enabled = !!(value & REG_CONTROL1_AIE);
+
+   err = pcf8523_read(client, REG_CONTROL2, );
+   if (err < 0)
+   return err;
+   tm->pending = !!(value & REG_CONTROL2_AF);
+
+   return 0;
+}
+
+static int pcf8523_irq_enable(struct device *dev, unsigned int enabled)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   u8 value;
+   int err;
+
+   err = pcf8523_read(client, REG_CONTROL1, );
+   if (err < 0)
+   return err;
+
+   value &= REG_CONTROL1_AIE;
+
+   if (enabled)
+   value |= REG_CONTROL1_AIE;
+
+   err = pcf8523_write(client, REG_CONTROL1, value);
+   if (err < 0)
+   return err;
+
+   return 0;
+}
+
+static int pcf8523_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   struct i2c_msg msg;
+   u8 regs[5];
+   int err;
+
+   err = pcf8523_irq_enable(dev, 0);
+   if (err)
+   return err;
+
+   err = pcf8523_write(client, REG_CONTROL2, 0);
+   if (err < 0)
+   return err;
+
+   /* The alarm has no seconds, round up to nearest minute */
+   if (tm->time.tm_sec) {
+   time64_t alarm_time = rtc_tm_to_time64(>time);
+
+   alarm_time += 60 - tm->time.tm_sec;
+   rtc_time64_to_tm(alarm_time, >time);
+   }
+
+   regs[0] = REG_MINUTE_ALARM;
+   regs[1] = bin2bcd(tm->time.tm_min);
+   regs[2] = bin2bcd(tm->time.tm_hour);
+   regs[3] = bin2bcd(tm->time.tm_mday);
+   regs[4] = ALARM_DIS;
+   msg.addr = client->addr;
+   msg.flags = 0;
+

[PATCH 3/3] rtc: pcf8523: report oscillator failures

2021-04-17 Thread Alexandre Belloni
Report oscillator failures and invalid date/time on RTC_VL_READ.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-pcf8523.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c
index feadab8e3bd3..bb23379bbfc7 100644
--- a/drivers/rtc/rtc-pcf8523.c
+++ b/drivers/rtc/rtc-pcf8523.c
@@ -406,6 +406,8 @@ static int pcf8523_rtc_ioctl(struct device *dev, unsigned 
int cmd,
 unsigned long arg)
 {
struct i2c_client *client = to_i2c_client(dev);
+   unsigned int flags = 0;
+   u8 value;
int ret;
 
switch (cmd) {
@@ -414,9 +416,16 @@ static int pcf8523_rtc_ioctl(struct device *dev, unsigned 
int cmd,
if (ret < 0)
return ret;
if (ret)
-   ret = RTC_VL_BACKUP_LOW;
+   flags |= RTC_VL_BACKUP_LOW;
+
+   ret = pcf8523_read(client, REG_SECONDS, );
+   if (ret < 0)
+   return ret;
+
+   if (value & REG_SECONDS_OS)
+   flags |= RTC_VL_DATA_INVALID;
 
-   return put_user(ret, (unsigned int __user *)arg);
+   return put_user(flags, (unsigned int __user *)arg);
 
default:
return -ENOIOCTLCMD;
-- 
2.30.2



[PATCH 1/3] rtc: pcf8523: remove useless define

2021-04-17 Thread Alexandre Belloni
Drop DRIVER_NAME as it is only used once

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-pcf8523.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8523.c b/drivers/rtc/rtc-pcf8523.c
index 5e1e7b2a8c9a..fe3ab41d8326 100644
--- a/drivers/rtc/rtc-pcf8523.c
+++ b/drivers/rtc/rtc-pcf8523.c
@@ -9,8 +9,6 @@
 #include 
 #include 
 
-#define DRIVER_NAME "rtc-pcf8523"
-
 #define REG_CONTROL1 0x00
 #define REG_CONTROL1_CAP_SEL BIT(7)
 #define REG_CONTROL1_STOPBIT(5)
@@ -373,7 +371,7 @@ MODULE_DEVICE_TABLE(of, pcf8523_of_match);
 
 static struct i2c_driver pcf8523_driver = {
.driver = {
-   .name = DRIVER_NAME,
+   .name = "rtc-pcf8523",
.of_match_table = of_match_ptr(pcf8523_of_match),
},
.probe = pcf8523_probe,
-- 
2.30.2



[PATCH v2 3/3] rtc: rtc_update_irq_enable: rework UIE emulation

2021-04-17 Thread Alexandre Belloni
Now that the core is aware of whether alarms are available, it is possible
to decide whether UIE emulation is required before actually trying to set
the alarm.

This greatly simplifies rtc_update_irq_enable because there is now only one
error value to track and is not relying on the return value of
__rtc_set_alarm anymore.

Tested-by: Łukasz Stelmach 
Signed-off-by: Alexandre Belloni 
---
Changes in v2:
 - Fix possible deadlock when using UIE emulation (no impact on Łukasz' test)
 - Remove rc

 drivers/rtc/interface.c | 34 ++
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index dcb34c73319e..9a2bd4947007 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -545,7 +545,7 @@ EXPORT_SYMBOL_GPL(rtc_alarm_irq_enable);
 
 int rtc_update_irq_enable(struct rtc_device *rtc, unsigned int enabled)
 {
-   int rc = 0, err;
+   int err;
 
err = mutex_lock_interruptible(>ops_lock);
if (err)
@@ -561,17 +561,21 @@ int rtc_update_irq_enable(struct rtc_device *rtc, 
unsigned int enabled)
if (rtc->uie_rtctimer.enabled == enabled)
goto out;
 
-   if (rtc->uie_unsupported) {
-   err = -EINVAL;
-   goto out;
+   if (rtc->uie_unsupported || !test_bit(RTC_FEATURE_ALARM, 
rtc->features)) {
+   mutex_unlock(>ops_lock);
+#ifdef CONFIG_RTC_INTF_DEV_UIE_EMUL
+   return rtc_dev_update_irq_enable_emul(rtc, enabled);
+#else
+   return -EINVAL;
+#endif
}
 
if (enabled) {
struct rtc_time tm;
ktime_t now, onesec;
 
-   rc = __rtc_read_time(rtc, );
-   if (rc)
+   err = __rtc_read_time(rtc, );
+   if (err)
goto out;
onesec = ktime_set(1, 0);
now = rtc_tm_to_ktime(tm);
@@ -585,24 +589,6 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned 
int enabled)
 out:
mutex_unlock(>ops_lock);
 
-   /*
-* __rtc_read_time() failed, this probably means that the RTC time has
-* never been set or less probably there is a transient error on the
-* bus. In any case, avoid enabling emulation has this will fail when
-* reading the time too.
-*/
-   if (rc)
-   return rc;
-
-#ifdef CONFIG_RTC_INTF_DEV_UIE_EMUL
-   /*
-* Enable emulation if the driver returned -EINVAL to signal that it has
-* been configured without interrupts or they are not available at the
-* moment.
-*/
-   if (err == -EINVAL)
-   err = rtc_dev_update_irq_enable_emul(rtc, enabled);
-#endif
return err;
 }
 EXPORT_SYMBOL_GPL(rtc_update_irq_enable);
-- 
2.30.2



[PATCH v2 2/3] rtc: ds1307: remove flags

2021-04-17 Thread Alexandre Belloni
flags is now unused, drop it.

Tested-by: Łukasz Stelmach 
Reviewed-by: Łukasz Stelmach 
Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds1307.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 76d67c419f7d..089509d0a3a0 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -169,8 +169,6 @@ enum ds_type {
 
 struct ds1307 {
enum ds_typetype;
-   unsigned long   flags;
-#define HAS_NVRAM  0   /* bit 0 == sysfs file active */
struct device   *dev;
struct regmap   *regmap;
const char  *name;
-- 
2.30.2



[PATCH v2 1/3] rtc: ds1307: replace HAS_ALARM by RTC_FEATURE_ALARM

2021-04-17 Thread Alexandre Belloni
The core now has RTC_FEATURE_ALARM for the driver to indicate whether
alarms are available. Use that instead of HAS_ALARM to ensure the alarm
callbacks are not even called.

Tested-by: Łukasz Stelmach 
Reviewed-by: Łukasz Stelmach 
Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds1307.c | 42 +++-
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index cd8e438bc9c4..76d67c419f7d 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -171,7 +171,6 @@ struct ds1307 {
enum ds_typetype;
unsigned long   flags;
 #define HAS_NVRAM  0   /* bit 0 == sysfs file active */
-#define HAS_ALARM  1   /* bit 1 == irq claimed */
struct device   *dev;
struct regmap   *regmap;
const char  *name;
@@ -411,9 +410,6 @@ static int ds1337_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
int ret;
u8  regs[9];
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* read all ALARM1, ALARM2, and status registers at once */
ret = regmap_bulk_read(ds1307->regmap, DS1339_REG_ALARM1_SECS,
   regs, sizeof(regs));
@@ -454,9 +450,6 @@ static int ds1337_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8  control, status;
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
dev_dbg(dev, "%s secs=%d, mins=%d, "
"hours=%d, mday=%d, enabled=%d, pending=%d\n",
"alarm set", t->time.tm_sec, t->time.tm_min,
@@ -512,9 +505,6 @@ static int ds1307_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
 {
struct ds1307   *ds1307 = dev_get_drvdata(dev);
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -ENOTTY;
-
return regmap_update_bits(ds1307->regmap, DS1337_REG_CONTROL,
  DS1337_BIT_A1IE,
  enabled ? DS1337_BIT_A1IE : 0);
@@ -592,9 +582,6 @@ static int rx8130_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 ald[3], ctl[3];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* Read alarm registers. */
ret = regmap_bulk_read(ds1307->regmap, RX8130_REG_ALARM_MIN, ald,
   sizeof(ald));
@@ -634,9 +621,6 @@ static int rx8130_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 ald[3], ctl[3];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
dev_dbg(dev, "%s, sec=%d min=%d hour=%d wday=%d mday=%d mon=%d "
"enabled=%d pending=%d\n", __func__,
t->time.tm_sec, t->time.tm_min, t->time.tm_hour,
@@ -681,9 +665,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
struct ds1307 *ds1307 = dev_get_drvdata(dev);
int ret, reg;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
ret = regmap_read(ds1307->regmap, RX8130_REG_CONTROL0, );
if (ret < 0)
return ret;
@@ -735,9 +716,6 @@ static int mcp794xx_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 regs[10];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* Read control and alarm 0 registers. */
ret = regmap_bulk_read(ds1307->regmap, MCP794XX_REG_CONTROL, regs,
   sizeof(regs));
@@ -793,9 +771,6 @@ static int mcp794xx_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
unsigned char regs[10];
int wday, ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
wday = mcp794xx_alm_weekday(dev, >time);
if (wday < 0)
return wday;
@@ -842,9 +817,6 @@ static int mcp794xx_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
 {
struct ds1307 *ds1307 = dev_get_drvdata(dev);
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
return regmap_update_bits(ds1307->regmap, MCP794XX_REG_CONTROL,
  MCP794XX_BIT_ALM0_EN,
  enabled ? MCP794XX_BIT_ALM0_EN : 0);
@@ -1641,7 +1613,7 @@ static int ds3231_clks_register(struct ds1307 *ds1307)
 * Interrupt signal due to alarm conditions and square-wave
 * output share same pin, so don't initialize both.
 */
-   if (i == DS3231_CLK_SQW && test_bit(HAS_ALARM, >flags))
+   if (i == DS3231_CLK_SQW &&

[PATCH] imx-sc: remove .read_alarm

2021-04-17 Thread Alexandre Belloni
The RTC core properly handles RTC without .read_alarm and doesn't use it to
set alarms. .read_alarm can be safely dropped.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-imx-sc.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/rtc/rtc-imx-sc.c b/drivers/rtc/rtc-imx-sc.c
index cc9fbab4..814d516645e2 100644
--- a/drivers/rtc/rtc-imx-sc.c
+++ b/drivers/rtc/rtc-imx-sc.c
@@ -80,16 +80,6 @@ static int imx_sc_rtc_alarm_irq_enable(struct device *dev, 
unsigned int enable)
return imx_scu_irq_group_enable(SC_IRQ_GROUP_RTC, SC_IRQ_RTC, enable);
 }
 
-static int imx_sc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
-{
-   /*
-* SCU firmware does NOT provide read alarm API, but .read_alarm
-* callback is required by RTC framework to support alarm function,
-* so just return here.
-*/
-   return 0;
-}
-
 static int imx_sc_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 {
struct imx_sc_msg_timer_rtc_set_alarm msg;
@@ -127,7 +117,6 @@ static int imx_sc_rtc_set_alarm(struct device *dev, struct 
rtc_wkalrm *alrm)
 static const struct rtc_class_ops imx_sc_rtc_ops = {
.read_time = imx_sc_rtc_read_time,
.set_time = imx_sc_rtc_set_time,
-   .read_alarm = imx_sc_rtc_read_alarm,
.set_alarm = imx_sc_rtc_set_alarm,
.alarm_irq_enable = imx_sc_rtc_alarm_irq_enable,
 };
-- 
2.30.2



Re: [PATCH] rtc: remove unused function

2021-04-16 Thread Alexandre Belloni
On Thu, 15 Apr 2021 16:37:01 +0800, Jiapeng Chong wrote:
> Fix the following clang warning:
> 
> drivers/rtc/rtc-ds1511.c:108:1: warning: unused function
> 'rtc_write_alarm' [-Wunused-function].

Applied, thanks!

[1/1] rtc: remove unused function
  commit: ef062a45ec9c2b6d15ddd9f76f897219334c7cd2

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH] rtc: fsl-ftm-alarm: add MODULE_TABLE()

2021-04-16 Thread Alexandre Belloni
On Wed, 14 Apr 2021 10:40:06 +0200, Michael Walle wrote:
> The module doesn't load automatically. Fix it by adding the missing
> MODULE_TABLE().

Applied, thanks!

[1/1] rtc: fsl-ftm-alarm: add MODULE_TABLE()
  commit: 199bb382375ad1b2178e250f82f6d95f8d6f7709

Best regards,
-- 
Alexandre Belloni 


Re: (subset) [PATCH V2 0/4] Add RTC support for PMIC PMK8350

2021-04-16 Thread Alexandre Belloni
On Fri, 9 Apr 2021 19:29:22 +0530, satya priya wrote:
> satya priya (4):
>   rtc: pm8xxx: Add RTC support for PMIC PMK8350
>   dt-bindings: mfd: Add compatible for pmk8350 rtc
>   dt-bindings: mfd: Convert pm8xxx bindings to yaml
>   dt-bindings: rtc: qcom-pm8xxx-rtc: Add qcom pm8xxx rtc bindings
> 
>  .../devicetree/bindings/mfd/qcom-pm8xxx.txt| 99 
> --
>  .../devicetree/bindings/mfd/qcom-pm8xxx.yaml   | 54 
>  .../devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml   | 62 ++
>  drivers/rtc/rtc-pm8xxx.c   | 11 +++
>  4 files changed, 127 insertions(+), 99 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
>  create mode 100644 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml

Applied, thanks!

[1/4] rtc: pm8xxx: Add RTC support for PMIC PMK8350
  commit: c8f0ca8b7a4b91f637ccd9a55f37dbac73d6f6bf
[4/4] dt-bindings: rtc: qcom-pm8xxx-rtc: Add qcom pm8xxx rtc bindings
  commit: 8138c5f0318c69a878582d2140dac08e6a99880d

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH 1/1] rtc: pcf85063: fallback to parent of_node

2021-04-16 Thread Alexandre Belloni
On Wed, 10 Mar 2021 16:10:26 -0500, Francois Gervais wrote:
> The rtc device node is always or at the very least can possibly be NULL.
> 
> Since v5.12-rc1-dontuse/3c9ea42802a1fbf7ef29660ff8c6e526c58114f6 this
> will lead to a NULL pointer dereference.
> 
> To fix this we fallback to using the parent node which is the i2c client
> node as set by devm_rtc_allocate_device().
> 
> [...]

Applied, thanks!

[1/1] rtc: pcf85063: fallback to parent of_node
  commit: 03531606ef4cda25b629f500d1ffb6173b805c05

I made the fallback unconditionnal because this should have been that way from
the beginning as you point out.

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH 1/1] rtc: pcf85063: add integrity check

2021-04-16 Thread Alexandre Belloni
Hi,

On 11/03/2021 12:49:40-0500, Francois Gervais wrote:
> Sometimes when the RTC battery is inserted, the voltage will bounce a
> bit and we've seen that this can randomly flip configuration bits in
> the RTC.
> 
> For example, we've seen COF bits flips and then the output clock
> frequency would not be the expected one anymore.
> 
> To remediate this issue, this adds an optional feature where if the OS
> bit it set on boot, it's possibly because the RTC lost power and again
> possibly because a new battery has been inserted. In that case, it
> reapplies defaults to configuration registers.
> 
> Signed-off-by: Francois Gervais 
> ---
>  drivers/rtc/rtc-pcf85063.c | 54 ++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
> index 463991c74fdd..774cc4cf93d8 100644
> --- a/drivers/rtc/rtc-pcf85063.c
> +++ b/drivers/rtc/rtc-pcf85063.c
> @@ -57,6 +57,10 @@
>  #define PCF85063_REG_ALM_S   0x0b
>  #define PCF85063_AEN BIT(7)
>  
> +static bool integrity_check;
> +module_param(integrity_check, bool, 0444);
> +MODULE_PARM_DESC(integrity_check, "Set to one to enable the integrity 
> check.");
> +
>  struct pcf85063_config {
>   struct regmap_config regmap;
>   unsigned has_alarms:1;
> @@ -357,6 +361,49 @@ static int pcf85063_load_capacitance(struct pcf85063 
> *pcf85063,
> PCF85063_REG_CTRL1_CAP_SEL, reg);
>  }
>  
> +static int pcf85063_check_integrity(struct pcf85063 *pcf85063)
> +{
> + int err;
> + unsigned int val;
> +
> + err = regmap_read(pcf85063->regmap, PCF85063_REG_SC, );
> + if (err < 0) {
> + dev_warn(>rtc->dev, "failed to read OS bit: %d",
> +  err);
> + return err;
> + }
> +
> + if (!(val & PCF85063_REG_SC_OS)) {
> + dev_dbg(>rtc->dev, "integrity is ok\n");
> + return 0;
> + }
> +
> + dev_dbg(>rtc->dev, "Power loss detected, restoring 
> defaults\n");
> + err = regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL1,
> +  (unsigned int)~PCF85063_REG_CTRL1_CAP_SEL, 0);
> + if (err < 0)
> + goto err_restore;
> +
> + err = regmap_write(pcf85063->regmap, PCF85063_REG_CTRL2, 0);
> + if (err < 0)
> + goto err_restore;
> +
> + err = regmap_write(pcf85063->regmap, PCF85063_REG_OFFSET, 0);
> + if (err < 0)
> + goto err_restore;
> +
> + err = regmap_write(pcf85063->regmap, PCF85063_REG_RAM, 0);
> + if (err < 0)
> + goto err_restore;
> +

I'm not sure I get the use case because PCF85063_REG_CTRL2 should be
initialized properly after the driver is probed anyway. The other two
can be set from userspace once it detects the oscillator failure which
would be better at deciding the policy anyway.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 0/2] m68k: Add Virtual M68k Machine

2021-04-16 Thread Alexandre Belloni
On 16/04/2021 22:26:26+0200, Alexandre Belloni wrote:
> On Tue, 23 Mar 2021 23:14:28 +0100, Laurent Vivier wrote:
> > The most powerful m68k machine emulated by QEMU is a Quadra 800,
> > but this machine is very limited: only 1 GiB of memory and only some
> > specific interfaces, with no DMA.
> > 
> > The Virtual M68k Machine is based on Goldfish interfaces defined by Google
> > for Android simulator. It uses Goldfish-rtc (timer and RTC),
> > Goldfish-pic (PIC) and Goldfish-tty (for early tty).
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/2] rtc: goldfish: remove dependency to OF
>   commit: 3fd00fdc4f11c656a63e6a6280c0bcb63cf109a2
> [2/2] m68k: introduce a virtual m68k machine
>   commit: 95631785c64840f3816f7a4cc2ce1a5332f43184
> 

Ah, obviously, I'm not applying the m68k patch.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 0/2] m68k: Add Virtual M68k Machine

2021-04-16 Thread Alexandre Belloni
On Tue, 23 Mar 2021 23:14:28 +0100, Laurent Vivier wrote:
> The most powerful m68k machine emulated by QEMU is a Quadra 800,
> but this machine is very limited: only 1 GiB of memory and only some
> specific interfaces, with no DMA.
> 
> The Virtual M68k Machine is based on Goldfish interfaces defined by Google
> for Android simulator. It uses Goldfish-rtc (timer and RTC),
> Goldfish-pic (PIC) and Goldfish-tty (for early tty).
> 
> [...]

Applied, thanks!

[1/2] rtc: goldfish: remove dependency to OF
  commit: 3fd00fdc4f11c656a63e6a6280c0bcb63cf109a2
[2/2] m68k: introduce a virtual m68k machine
  commit: 95631785c64840f3816f7a4cc2ce1a5332f43184

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH V2 3/4] dt-bindings: mfd: Convert pm8xxx bindings to yaml

2021-04-16 Thread Alexandre Belloni
Hi,

On 16/04/2021 12:20:30-0500, Rob Herring wrote:
> On Wed, Apr 14, 2021 at 3:38 AM Lee Jones  wrote:
> >
> > On Fri, 09 Apr 2021, satya priya wrote:
> >
> > > Convert pm8xxx bindings from .txt to .yaml format. Also,
> > > split this binding into two: parent binding(qcom-pm8xxx.yaml)
> > > and child node RTC binding(qcom-pm8xxx-rtc.yaml).
> > >
> > > Signed-off-by: satya priya 
> > > ---
> > > Changes in V2:
> > >  - As per Bjorn's comments, I've split this into two, one parent binding
> > >and one child node rtc binding.
> > >  - Fixed bot errors and changed maintainer name.
> > >
> > >  .../devicetree/bindings/mfd/qcom-pm8xxx.txt| 100 
> > > -
> > >  .../devicetree/bindings/mfd/qcom-pm8xxx.yaml   |  54 +++
> > >  2 files changed, 54 insertions(+), 100 deletions(-)
> > >  delete mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
> >
> > Applied, thanks.
> 
> You need to apply the rtc schema too. linux-next has an error on this one now.
> 

I'm going to apply it later tonight


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH V2 0/4] Add RTC support for PMIC PMK8350

2021-04-13 Thread Alexandre Belloni
Lee,

On 09/04/2021 19:29:22+0530, satya priya wrote:
> satya priya (4):
>   rtc: pm8xxx: Add RTC support for PMIC PMK8350
>   dt-bindings: mfd: Add compatible for pmk8350 rtc
>   dt-bindings: mfd: Convert pm8xxx bindings to yaml
>   dt-bindings: rtc: qcom-pm8xxx-rtc: Add qcom pm8xxx rtc bindings
> 
>  .../devicetree/bindings/mfd/qcom-pm8xxx.txt| 99 
> --
>  .../devicetree/bindings/mfd/qcom-pm8xxx.yaml   | 54 
>  .../devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml   | 62 ++
>  drivers/rtc/rtc-pm8xxx.c   | 11 +++
>  4 files changed, 127 insertions(+), 99 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/qcom-pm8xxx.yaml
>  create mode 100644 Documentation/devicetree/bindings/rtc/qcom-pm8xxx-rtc.yaml
> 

This is mostly about the RTC driver but the bindings doc is in mfd. How
do you prefer that to be merged? there is no build dependency so I can
take 1 and 4 and you could take 2 and 3. Or one of us can take all 4
patches.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 22/24] ARM: at91: sama7: introduce sama7 SoC family

2021-04-08 Thread Alexandre Belloni
On 08/04/2021 17:24:39+0200, Nicolas Ferre wrote:
> On 01/04/2021 at 12:24, Claudiu Beznea - M18063 wrote:
> > On 01.04.2021 12:38, Claudiu Beznea - M18063 wrote:
> > > On 31.03.2021 19:01, Alexandre Belloni wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know 
> > > > the content is safe
> > > > 
> > > > On 31/03/2021 13:59:06+0300, Claudiu Beznea wrote:
> > > > > From: Eugen Hristev 
> > > > > 
> > > > > Introduce new family of SoCs, sama7, and first SoC, sama7g5.
> > > > > 
> > > > > Signed-off-by: Eugen Hristev 
> > > > > Signed-off-by: Claudiu Beznea 
> > > > > ---
> > > > >   arch/arm/mach-at91/Makefile |  1 +
> > > > >   arch/arm/mach-at91/sama7.c  | 48 
> > > > > +
> > > > >   2 files changed, 49 insertions(+)
> > > > >   create mode 100644 arch/arm/mach-at91/sama7.c
> > > > > 
> > > > > diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> > > > > index f565490f1b70..6cc6624cddac 100644
> > > > > --- a/arch/arm/mach-at91/Makefile
> > > > > +++ b/arch/arm/mach-at91/Makefile
> > > > > @@ -9,6 +9,7 @@ obj-$(CONFIG_SOC_AT91SAM9)+= at91sam9.o
> > > > >   obj-$(CONFIG_SOC_SAM9X60)+= sam9x60.o
> > > > >   obj-$(CONFIG_SOC_SAMA5)  += sama5.o
> > > > >   obj-$(CONFIG_SOC_SAMV7)  += samv7.o
> > > > > +obj-$(CONFIG_SOC_SAMA7)  += sama7.o
> > > > > 
> > > > >   # Power Management
> > > > >   obj-$(CONFIG_ATMEL_PM)   += pm.o pm_suspend.o
> > > > > diff --git a/arch/arm/mach-at91/sama7.c b/arch/arm/mach-at91/sama7.c
> > > > > new file mode 100644
> > > > > index ..e04cadb569ad
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-at91/sama7.c
> > > > > @@ -0,0 +1,48 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > > +/*
> > > > > + * Setup code for SAMA7
> > > > > + *
> > > > > + * Copyright (C) 2021 Microchip Technology, Inc. and its subsidiaries
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +#include "generic.h"
> > > > > +
> > > > > +static void __init sama7_common_init(void)
> > > > > +{
> > > > > + of_platform_default_populate(NULL, NULL, NULL);
> > > > 
> > > > Is this necessary? This is left as a workaround for the old SoCs using
> > > > pinctrl-at91. I guess this will be using pio4 so this has to be removed.
> > > 
> > > OK, I'll have a look. BTW, SAMA5D2 which is also using PIO4 calls
> > > of_platform_default_populate(NULL, NULL, NULL);
> > 
> > Without this call the PM code (arch/arm/mach-at/pm.c) is not able to locate
> > proper DT nodes:
> > 
> > [0.194615] at91_pm_backup_init: failed to find securam device!
> > [0.201393] at91_pm_sram_init: failed to find sram device!
> > [0.207449] AT91: PM not supported, due to no SRAM allocated
> 
> Okay, so we can't afford removing these calls to sama5d2 and upcoming
> sama7g5 right now.
> 
> Is it a common pattern to have to reach DT content in the early stages that
> explicit call to of_platform_default_populate() tries to solve?
> 

That's fine, I didn't remember about that one, we can keep the call.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: New 'make dtbs_check W=1' warnings

2021-04-08 Thread Alexandre Belloni
Hi,

On 08/04/2021 17:08:26+0200, Arnd Bergmann wrote:
> arch/arm/boot/dts/at91-sama5d2_ptc_ek.dt.yaml: /: 'etm@73C000' does
> not match any of the regexes: '@(0|[1-9a-f][0-9a-f]*)$', '^[^@]+$',
> 'pinctrl-[0-9]+'
> arch/arm/boot/dts/at91-kizbox3-hs.dt.yaml: /: 'etm@73C000' does not
> match any of the regexes: '@(0|[1-9a-f][0-9a-f]*)$', '^[^@]+$',
> 'pinctrl-[0-9]+'
> 

This was introduced by 4d930c421e3b ("ARM: dts: at91: sama5d2: add ETB
and ETM unit name"), trying to fix another warning.

I guess this is because
Documentation/devicetree/bindings/arm/coresight.txt is not yaml yet.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 3/3] ARM: at91: Kconfig: select PLL, generic clock and utmi support

2021-04-07 Thread Alexandre Belloni
Hi,

On 07/04/2021 20:00:53+0300, Eugen Hristev wrote:
> From: Claudiu Beznea 
> 
> Select PLL, generic clock and UTMI support for SAMA7G5.
> 
> Signed-off-by: Claudiu Beznea 
> Signed-off-by: Eugen Hristev 
> ---
>  arch/arm/mach-at91/Kconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> index 5eb2a9206f42..f52b46bccd85 100644
> --- a/arch/arm/mach-at91/Kconfig
> +++ b/arch/arm/mach-at91/Kconfig
> @@ -60,6 +60,9 @@ config SOC_SAMA5D4
>  config SOC_SAMA7G5
>   bool "SAMA7G5 family"
>   depends on ARCH_MULTI_V7
> + select HAVE_AT91_GENERATED_CLK
> + select HAVE_AT91_SAM9X60_PLL
> + select HAVE_AT91_UTMI

Shouldn't that be squashed in 1/3?

>   select SOC_SAMA7
>   help
> Select this if you are using one of Microchip's SAMA7G5 family SoC.
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] memory: atmel-sdramc: check of_device_get_match_data() return value

2021-04-07 Thread Alexandre Belloni
Hi,

On 07/04/2021 17:44:47+0200, Krzysztof Kozlowski wrote:
> If the driver is probed, the of_device_get_match_data() should not
> return NULL, however for sanity check its return value.
> 

As you say, there is no way this will ever be the case, I don't see the
point of checking the return value, this adds 12 bytes for nothing...

Maybe coverity should be fixed as there are many drivers making the same
(true) assumption and I don't think this is worth the churn.

> Addresses-Coverity: Dereference null return value
> Signed-off-by: Krzysztof Kozlowski 
> ---
>  drivers/memory/atmel-sdramc.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/memory/atmel-sdramc.c b/drivers/memory/atmel-sdramc.c
> index 9c49d00c2a96..e09b2617f63d 100644
> --- a/drivers/memory/atmel-sdramc.c
> +++ b/drivers/memory/atmel-sdramc.c
> @@ -45,6 +45,8 @@ static int atmel_ramc_probe(struct platform_device *pdev)
>   struct clk *clk;
>  
>   caps = of_device_get_match_data(>dev);
> + if (!caps)
> + return -EINVAL;
>  
>   if (caps->has_ddrck) {
>   clk = devm_clk_get(>dev, "ddrck");
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v2 09/19] dt-bindings: i3c: update i3c.yaml references

2021-04-07 Thread Alexandre Belloni
On 07/04/2021 10:20:48+0200, Mauro Carvalho Chehab wrote:
> Changeset 5e4cdca887fd ("dt-bindings: i3c: Convert the bus description to 
> yaml")
> renamed: Documentation/devicetree/bindings/i3c/i3c.txt
> to: Documentation/devicetree/bindings/i3c/i3c.yaml.
> 
> Update the cross-references accordingly.
> 
> Fixes: 5e4cdca887fd ("dt-bindings: i3c: Convert the bus description to yaml")
> Acked-by: Rob Herring 
Acked-by: Alexandre Belloni 

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt   | 6 +++---
>  .../devicetree/bindings/i3c/snps,dw-i3c-master.txt  | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt 
> b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> index 1cf6182f888c..3716589d6999 100644
> --- a/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> +++ b/Documentation/devicetree/bindings/i3c/cdns,i3c-master.txt
> @@ -10,19 +10,19 @@ Required properties:
>  - reg: I3C master registers
>  
>  Mandatory properties defined by the generic binding (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details):
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details):
>  
>  - #address-cells: shall be set to 1
>  - #size-cells: shall be set to 0
>  
>  Optional properties defined by the generic binding (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details):
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details):
>  
>  - i2c-scl-hz
>  - i3c-scl-hz
>  
>  I3C device connected on the bus follow the generic description (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details).
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details).
>  
>  Example:
>  
> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt 
> b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt
> index 5020eb71eb8d..07f35f36085d 100644
> --- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt
> +++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.txt
> @@ -9,19 +9,19 @@ Required properties:
>  - reg: Offset and length of I3C master registers
>  
>  Mandatory properties defined by the generic binding (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details):
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details):
>  
>  - #address-cells: shall be set to 3
>  - #size-cells: shall be set to 0
>  
>  Optional properties defined by the generic binding (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details):
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details):
>  
>  - i2c-scl-hz
>  - i3c-scl-hz
>  
>  I3C device connected on the bus follow the generic description (see
> -Documentation/devicetree/bindings/i3c/i3c.txt for more details).
> +Documentation/devicetree/bindings/i3c/i3c.yaml for more details).
>  
>  Example:
>  
> -- 
> 2.30.2
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 22/24] ARM: at91: sama7: introduce sama7 SoC family

2021-03-31 Thread Alexandre Belloni
On 31/03/2021 13:59:06+0300, Claudiu Beznea wrote:
> From: Eugen Hristev 
> 
> Introduce new family of SoCs, sama7, and first SoC, sama7g5.
> 
> Signed-off-by: Eugen Hristev 
> Signed-off-by: Claudiu Beznea 
> ---
>  arch/arm/mach-at91/Makefile |  1 +
>  arch/arm/mach-at91/sama7.c  | 48 +
>  2 files changed, 49 insertions(+)
>  create mode 100644 arch/arm/mach-at91/sama7.c
> 
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index f565490f1b70..6cc6624cddac 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_SOC_AT91SAM9)+= at91sam9.o
>  obj-$(CONFIG_SOC_SAM9X60)+= sam9x60.o
>  obj-$(CONFIG_SOC_SAMA5)  += sama5.o
>  obj-$(CONFIG_SOC_SAMV7)  += samv7.o
> +obj-$(CONFIG_SOC_SAMA7)  += sama7.o
>  
>  # Power Management
>  obj-$(CONFIG_ATMEL_PM)   += pm.o pm_suspend.o
> diff --git a/arch/arm/mach-at91/sama7.c b/arch/arm/mach-at91/sama7.c
> new file mode 100644
> index ..e04cadb569ad
> --- /dev/null
> +++ b/arch/arm/mach-at91/sama7.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Setup code for SAMA7
> + *
> + * Copyright (C) 2021 Microchip Technology, Inc. and its subsidiaries
> + *
> + */
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +
> +#include "generic.h"
> +
> +static void __init sama7_common_init(void)
> +{
> + of_platform_default_populate(NULL, NULL, NULL);

Is this necessary? This is left as a workaround for the old SoCs using
pinctrl-at91. I guess this will be using pio4 so this has to be removed.

> +}
> +
> +static void __init sama7_dt_device_init(void)
> +{
> + sama7_common_init();
> +}
> +
> +static const char *const sama7_dt_board_compat[] __initconst = {
> + "microchip,sama7",
> + NULL
> +};
> +
> +DT_MACHINE_START(sama7_dt, "Microchip SAMA7")
> + /* Maintainer: Microchip */
> + .init_machine   = sama7_dt_device_init,
> + .dt_compat  = sama7_dt_board_compat,
> +MACHINE_END
> +
> +static const char *const sama7g5_dt_board_compat[] __initconst = {
> + "microchip,sama7g5",
> + NULL
> +};
> +
> +DT_MACHINE_START(sama7g5_dt, "Microchip SAMA7G5")
> + /* Maintainer: Microchip */
> + .init_machine   = sama7_dt_device_init,
> + .dt_compat  = sama7g5_dt_board_compat,
> +MACHINE_END
> +
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 10/24] ARM: at91: sfrbu: add sfrbu registers definitions for sama7g5

2021-03-31 Thread Alexandre Belloni
On 31/03/2021 13:58:54+0300, Claudiu Beznea wrote:
> Add SFRBU registers definitions for SAMA7G5.
> 
> Signed-off-by: Claudiu Beznea 
> ---
>  include/soc/at91/sama7-sfrbu.h | 34 ++
>  1 file changed, 34 insertions(+)
>  create mode 100644 include/soc/at91/sama7-sfrbu.h
> 
> diff --git a/include/soc/at91/sama7-sfrbu.h b/include/soc/at91/sama7-sfrbu.h
> new file mode 100644
> index ..76b740810d34
> --- /dev/null
> +++ b/include/soc/at91/sama7-sfrbu.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Microchip SAMA7 SFRBU registers offsets and bit definitions.
> + *
> + * Copyright (C) [2020] Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Claudu Beznea 
> + */
> +
> +#ifndef __SAMA7_SFRBU_H__
> +#define __SAMA7_SFRBU_H__
> +
> +#ifdef CONFIG_SOC_SAMA7
> +
> +#define AT91_SFRBU_PSWBU (0x00)  /* SFRBU Power 
> Switch BU Control Register */
> +#define  AT91_SFRBU_PSWBU_PSWKEY (0x4BD20C << 8) /* 
> Specific value mandatory to allow writing of other register bits */
> +#define  AT91_SFRBU_PSWBU_STATE  (1 << 2)/* 
> Power switch BU state */
> +#define  AT91_SFRBU_PSWBU_SOFTSWITCH (1 << 1)/* 
> Power switch BU source selection */
> +#define  AT91_SFRBU_PSWBU_CTRL   (1 << 0)/* 
> Power switch BU control */

Please use BIT

> +
> +#define AT91_SFRBU_25LDOCR   (0x0C)  /* SFRBU 2.5V 
> LDO Control Register */
> +#define  AT91_SFRBU_25LDOCR_LDOANAKEY(0x3B6E18 << 8) /* 
> Specific value mandatory to allow writing of other register bits. */
> +#define  AT91_SFRBU_25LDOCR_STATE(1 << 3)/* 
> LDOANA Switch On/Off Control */
> +#define  AT91_SFRBU_25LDOCR_LP   (1 << 2)/* 
> LDOANA Low-Power Mode Control */
> +#define  AT91_SFRBU_PD_VALUE_MSK (0x3)

GENMASK

> +#define  AT91_SFRBU_25LDOCR_PD_VALUE(v)  ((v) & 
> AT91_SFRBU_PD_VALUE_MSK) /* LDOANA Pull-down value */

this macro is not necessary, you can use FIELD_PREP with the previous
define.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 01/24] ARM: at91: pm: move pm_bu to soc_pm data structure

2021-03-31 Thread Alexandre Belloni
On 31/03/2021 13:58:45+0300, Claudiu Beznea wrote:
> Move pm_bu to soc_pm data structure.
> 
> Signed-off-by: Claudiu Beznea 
Reviewed-by: Alexandre Belloni 

> ---
>  arch/arm/mach-at91/pm.c | 34 +-
>  1 file changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 90dcdfe3b3d0..e13ceef7ac9a 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -27,10 +27,25 @@
>  #include "generic.h"
>  #include "pm.h"
>  
> +/**
> + * struct at91_pm_bu - AT91 power management backup unit data structure
> + * @suspended: true if suspended to backup mode
> + * @reserved: reserved
> + * @canary: canary data for memory checking after exit from backup mode
> + * @resume: resume API
> + */
> +struct at91_pm_bu {
> + int suspended;
> + unsigned long reserved;
> + phys_addr_t canary;
> + phys_addr_t resume;
> +};
> +
>  struct at91_soc_pm {
>   int (*config_shdwc_ws)(void __iomem *shdwc, u32 *mode, u32 *polarity);
>   int (*config_pmc_ws)(void __iomem *pmc, u32 mode, u32 polarity);
>   const struct of_device_id *ws_ids;
> + struct at91_pm_bu *bu;
>   struct at91_pm_data data;
>  };
>  
> @@ -71,13 +86,6 @@ static int at91_pm_valid_state(suspend_state_t state)
>  
>  static int canary = 0xA5A5A5A5;
>  
> -static struct at91_pm_bu {
> - int suspended;
> - unsigned long reserved;
> - phys_addr_t canary;
> - phys_addr_t resume;
> -} *pm_bu;
> -
>  struct wakeup_source_info {
>   unsigned int pmc_fsmr_bit;
>   unsigned int shdwc_mr_bit;
> @@ -288,7 +296,7 @@ static int at91_suspend_finish(unsigned long val)
>  static void at91_pm_suspend(suspend_state_t state)
>  {
>   if (soc_pm.data.mode == AT91_PM_BACKUP) {
> - pm_bu->suspended = 1;
> + soc_pm.bu->suspended = 1;
>  
>   cpu_suspend(0, at91_suspend_finish);
>  
> @@ -657,16 +665,16 @@ static int __init at91_pm_backup_init(void)
>   goto securam_fail;
>   }
>  
> - pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
> - if (!pm_bu) {
> + soc_pm.bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct 
> at91_pm_bu));
> + if (!soc_pm.bu) {
>   pr_warn("%s: unable to alloc securam!\n", __func__);
>   ret = -ENOMEM;
>   goto securam_fail;
>   }
>  
> - pm_bu->suspended = 0;
> - pm_bu->canary = __pa_symbol();
> - pm_bu->resume = __pa_symbol(cpu_resume);
> + soc_pm.bu->suspended = 0;
> + soc_pm.bu->canary = __pa_symbol();
> + soc_pm.bu->resume = __pa_symbol(cpu_resume);
>  
>   return 0;
>  
> -- 
> 2.25.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[PATCH 3/3] rtc: rtc_update_irq_enable: rework UIE emulation

2021-03-29 Thread Alexandre Belloni
Now that the core is aware of whether alarms are available, it is possible
to decide whether UIE emulation is required before actually trying to set
the alarm.

This greatly simplifies rtc_update_irq_enable because there is now only one
error value to track and is not relying on the return value of
__rtc_set_alarm anymore.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/interface.c | 28 +++-
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index dcb34c73319e..b162964d2b39 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -561,8 +561,12 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned 
int enabled)
if (rtc->uie_rtctimer.enabled == enabled)
goto out;
 
-   if (rtc->uie_unsupported) {
+   if (rtc->uie_unsupported || !test_bit(RTC_FEATURE_ALARM, 
rtc->features)) {
+#ifdef CONFIG_RTC_INTF_DEV_UIE_EMUL
+   err = rtc_dev_update_irq_enable_emul(rtc, enabled);
+#else
err = -EINVAL;
+#endif
goto out;
}
 
@@ -570,8 +574,8 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned 
int enabled)
struct rtc_time tm;
ktime_t now, onesec;
 
-   rc = __rtc_read_time(rtc, );
-   if (rc)
+   err = __rtc_read_time(rtc, );
+   if (err)
goto out;
onesec = ktime_set(1, 0);
now = rtc_tm_to_ktime(tm);
@@ -585,24 +589,6 @@ int rtc_update_irq_enable(struct rtc_device *rtc, unsigned 
int enabled)
 out:
mutex_unlock(>ops_lock);
 
-   /*
-* __rtc_read_time() failed, this probably means that the RTC time has
-* never been set or less probably there is a transient error on the
-* bus. In any case, avoid enabling emulation has this will fail when
-* reading the time too.
-*/
-   if (rc)
-   return rc;
-
-#ifdef CONFIG_RTC_INTF_DEV_UIE_EMUL
-   /*
-* Enable emulation if the driver returned -EINVAL to signal that it has
-* been configured without interrupts or they are not available at the
-* moment.
-*/
-   if (err == -EINVAL)
-   err = rtc_dev_update_irq_enable_emul(rtc, enabled);
-#endif
return err;
 }
 EXPORT_SYMBOL_GPL(rtc_update_irq_enable);
-- 
2.30.2



[PATCH 2/3] rtc: ds1307: remove flags

2021-03-29 Thread Alexandre Belloni
flags is now unused, drop it.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds1307.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 76d67c419f7d..089509d0a3a0 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -169,8 +169,6 @@ enum ds_type {
 
 struct ds1307 {
enum ds_typetype;
-   unsigned long   flags;
-#define HAS_NVRAM  0   /* bit 0 == sysfs file active */
struct device   *dev;
struct regmap   *regmap;
const char  *name;
-- 
2.30.2



[PATCH 1/3] rtc: ds1307: replace HAS_ALARM by RTC_FEATURE_ALARM

2021-03-29 Thread Alexandre Belloni
The core now has RTC_FEATURE_ALARM for the driver to indicate whether
alarms are available. Use that instead of HAS_ALARM to ensure the alarm
callbacks are not even called.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds1307.c | 42 +++-
 1 file changed, 7 insertions(+), 35 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index cd8e438bc9c4..76d67c419f7d 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -171,7 +171,6 @@ struct ds1307 {
enum ds_typetype;
unsigned long   flags;
 #define HAS_NVRAM  0   /* bit 0 == sysfs file active */
-#define HAS_ALARM  1   /* bit 1 == irq claimed */
struct device   *dev;
struct regmap   *regmap;
const char  *name;
@@ -411,9 +410,6 @@ static int ds1337_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
int ret;
u8  regs[9];
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* read all ALARM1, ALARM2, and status registers at once */
ret = regmap_bulk_read(ds1307->regmap, DS1339_REG_ALARM1_SECS,
   regs, sizeof(regs));
@@ -454,9 +450,6 @@ static int ds1337_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8  control, status;
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
dev_dbg(dev, "%s secs=%d, mins=%d, "
"hours=%d, mday=%d, enabled=%d, pending=%d\n",
"alarm set", t->time.tm_sec, t->time.tm_min,
@@ -512,9 +505,6 @@ static int ds1307_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
 {
struct ds1307   *ds1307 = dev_get_drvdata(dev);
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -ENOTTY;
-
return regmap_update_bits(ds1307->regmap, DS1337_REG_CONTROL,
  DS1337_BIT_A1IE,
  enabled ? DS1337_BIT_A1IE : 0);
@@ -592,9 +582,6 @@ static int rx8130_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 ald[3], ctl[3];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* Read alarm registers. */
ret = regmap_bulk_read(ds1307->regmap, RX8130_REG_ALARM_MIN, ald,
   sizeof(ald));
@@ -634,9 +621,6 @@ static int rx8130_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 ald[3], ctl[3];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
dev_dbg(dev, "%s, sec=%d min=%d hour=%d wday=%d mday=%d mon=%d "
"enabled=%d pending=%d\n", __func__,
t->time.tm_sec, t->time.tm_min, t->time.tm_hour,
@@ -681,9 +665,6 @@ static int rx8130_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
struct ds1307 *ds1307 = dev_get_drvdata(dev);
int ret, reg;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
ret = regmap_read(ds1307->regmap, RX8130_REG_CONTROL0, );
if (ret < 0)
return ret;
@@ -735,9 +716,6 @@ static int mcp794xx_read_alarm(struct device *dev, struct 
rtc_wkalrm *t)
u8 regs[10];
int ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
/* Read control and alarm 0 registers. */
ret = regmap_bulk_read(ds1307->regmap, MCP794XX_REG_CONTROL, regs,
   sizeof(regs));
@@ -793,9 +771,6 @@ static int mcp794xx_set_alarm(struct device *dev, struct 
rtc_wkalrm *t)
unsigned char regs[10];
int wday, ret;
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
wday = mcp794xx_alm_weekday(dev, >time);
if (wday < 0)
return wday;
@@ -842,9 +817,6 @@ static int mcp794xx_alarm_irq_enable(struct device *dev, 
unsigned int enabled)
 {
struct ds1307 *ds1307 = dev_get_drvdata(dev);
 
-   if (!test_bit(HAS_ALARM, >flags))
-   return -EINVAL;
-
return regmap_update_bits(ds1307->regmap, MCP794XX_REG_CONTROL,
  MCP794XX_BIT_ALM0_EN,
  enabled ? MCP794XX_BIT_ALM0_EN : 0);
@@ -1641,7 +1613,7 @@ static int ds3231_clks_register(struct ds1307 *ds1307)
 * Interrupt signal due to alarm conditions and square-wave
 * output share same pin, so don't initialize both.
 */
-   if (i == DS3231_CLK_SQW && test_bit(HAS_ALARM, >flags))
+   if (i == DS3231_CLK_SQW && test_bit(RTC_FEATURE_ALARM, 
ds1307->rtc->feature

Re: [PATCH] rtc: ds1307: set uie_unsupported if no interrupt is available

2021-03-29 Thread Alexandre Belloni
On 16/03/2021 19:04:14+0100, Lukasz Stelmach wrote:
> OK, you are right. The problem seems to be elsewhere.
> 
> How about this scnario? We call rtc_update_irq_enable(). We read rtc
> with __rtc_read_time() and calculate the alarm time. We get through
> rtc_timer_enqueue() and down to __rtc_set_alarm(). We loose the race
> condition (I can do it, I've got really slow connection to DS3231) and
> we return -ETIME from __rtc_set_alarm()
> 
> if (scheduled <= now)
> return -ETIME;
> 
> and 0 from rtc_timer_enqueue() and the very same zero from
> rtc_update_irq_enable(). The caller of ioctl() thinks they can expect
> interrupts when, in fact, they won't receive any.
> 
> The really weird stuff happens in rtc_timer_do_work(). For the timer to
> be dequeued __rtc_set_alarm() needs to return EINVAL three times in a
> row. In my setup this doesn't happen and the code keeps running loops
> around "reporogram" and "again" labels.
> 
> With my patch we never risk the above race condition between
> __rtc_read_time() in rtc_update_irq_enable() and the one in
> __rtc_set_alarm(), because we know rtc doesn't support alarms before we
> start the race. In fact there is another race between __rtc_read_time()
> and actually setting the alarm in the chip.
> 
> IMHO the solution is to introduce RTC_HAS_ALARM flag for struct
> rtc_device and check it at the very beginning of __rtc_set_alarm() the
> same way it is being done in ds1337_set_alarm(). What are your thoughts?
> 

I did introduce RTC_FEATURE_ALARM for that in v5.12. I'm sending patches
that are not well tested but should solve your issue.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH 0/4] rtc:abx80x: Enable distributed digital calibration

2021-03-28 Thread Alexandre Belloni
Hello,

Thank you for working on that!

On 29/03/2021 00:02:28+0300, Kirill Kapranov wrote:
> This patch series enables a Distributed Digital Calibration function for
> the RTC of the family. This feature allows to improve the RTC accuracy by
> means of compensation an XT oscillator drift. To learn more, see:
> AB08XX Series Ultra Low Power RTC IC User's Guide
> https://abracon.com/realtimeclock/AB08XX-Application-Manual.pdf
> 
> The patches 1 and 2 enable SQW output, that is necessary for subsequent
> measurement and computation. However, this feature may be enabled and used
> independently, as is.
> 

Please use the common clock framework for this part. You will need a
dummy userspace user for this to work but I know Stephen is open to that
as we discussed that use case multiple times already.

> The patches 3 and 4 enable the XT calibration feature per se. The SQW
> output must be enabled for usage of this feature.

Please use the .set_offset and .get_offset rtc_ops for this part.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [RESEND 1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header

2021-03-26 Thread Alexandre Belloni
On 26/03/2021 18:18:04+0100, Alexandre Belloni wrote:
> On Wed, 3 Mar 2021 12:41:49 +, Lee Jones wrote:
> > Both the caller and the supplier's source file should have access to
> > the include file containing the prototypes.
> > 
> > Fixes the following W=1 kernel build warning(s):
> > 
> >  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for 
> > ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]
> >  1637 | void at91_pinctrl_gpio_suspend(void)
> >  | ^
> >  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for 
> > ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]
> >  1661 | void at91_pinctrl_gpio_resume(void)
> >  | ^~~~
> 
> Applied, thanks!
> 
> [1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header
>   commit: 10e9119e865047f4e22cbd69de991d6bc26c4faf
> 

Actually:
[1/1] ARM: at91: pm: Move prototypes to mutually included header
  commit: 41dbf4a146a06443d1cbf39e238f02fa1ca9d626


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [RESEND 1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header

2021-03-26 Thread Alexandre Belloni
On Wed, 3 Mar 2021 12:41:49 +, Lee Jones wrote:
> Both the caller and the supplier's source file should have access to
> the include file containing the prototypes.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for 
> ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]
>  1637 | void at91_pinctrl_gpio_suspend(void)
>  | ^
>  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for 
> ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]
>  1661 | void at91_pinctrl_gpio_resume(void)
>  | ^~~~

Applied, thanks!

[1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header
  commit: 10e9119e865047f4e22cbd69de991d6bc26c4faf

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings

2021-03-25 Thread Alexandre Belloni
On 24/03/2021 17:46:19+0800, heying (H) wrote:
> Many thanks for your suggestion. As you suggest, rtc_lock should be local to
> platforms.
> 
> Does it mean not only powerpc but also all other platforms should adapt this
> change?

Not all the other ones, in the current state, x86 still needs it. I'll
work on that. Again, the patch is fine as is.

Reviewed-by: Alexandre Belloni 



-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH V3 -next] powerpc: kernel/time.c - cleanup warnings

2021-03-24 Thread Alexandre Belloni
On 24/03/2021 05:09:39-0400, He Ying wrote:
> We found these warnings in arch/powerpc/kernel/time.c as follows:
> warning: symbol 'decrementer_max' was not declared. Should it be static?
> warning: symbol 'rtc_lock' was not declared. Should it be static?
> warning: symbol 'dtl_consumer' was not declared. Should it be static?
> 
> Declare 'decrementer_max' in powerpc asm/time.h.
> Include linux/mc146818rtc.h in powerpc kernel/time.c where 'rtc_lock'
> is declared. And remove duplicated declaration of 'rtc_lock' in powerpc
> platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> Move 'dtl_consumer' definition behind "include " because it
> is declared there.
> 
> Reported-by: Hulk Robot 
> Signed-off-by: He Ying 
> ---
> V2:
> - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
>   rtc_lock in powerpc asm/time.h.
> V3:
> - Recover to V1, that is including linux/mc146818rtc.h in powerpc
>   kernel/time.c. And remove duplicated declaration of 'rtc_lock' in powerpc
>   platforms/chrp/time.c because it has included linux/mc146818rtc.h.
> 
>  arch/powerpc/include/asm/time.h| 1 +
>  arch/powerpc/kernel/time.c | 9 -
>  arch/powerpc/platforms/chrp/time.c | 2 --
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 8dd3cdb25338..2cd2b50bedda 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -22,6 +22,7 @@ extern unsigned long tb_ticks_per_jiffy;
>  extern unsigned long tb_ticks_per_usec;
>  extern unsigned long tb_ticks_per_sec;
>  extern struct clock_event_device decrementer_clockevent;
> +extern u64 decrementer_max;
>  
>  
>  extern void generic_calibrate_decr(void);
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index b67d93a609a2..ac81f043bf49 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -55,8 +55,9 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 

I'm fine with that but I really think my suggestion to make the rtc_lock
local to the platforms was better because it is only used to synchronize
between concurrent invocations of chrp_set_rtc_time or
maple_set_rtc_time. The rtc core will never do that and the only case
would be concurrent calls to rtc_ops.set_time and
update_persistent_clock64 (which should also be removed at some point).

>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -150,10 +151,6 @@ bool tb_invalid;
>  u64 __cputime_usec_factor;
>  EXPORT_SYMBOL(__cputime_usec_factor);
>  
> -#ifdef CONFIG_PPC_SPLPAR
> -void (*dtl_consumer)(struct dtl_entry *, u64);
> -#endif
> -
>  static void calc_cputime_factors(void)
>  {
>   struct div_result res;
> @@ -179,6 +176,8 @@ static inline unsigned long read_spurr(unsigned long tb)
>  
>  #include 
>  
> +void (*dtl_consumer)(struct dtl_entry *, u64);
> +
>  /*
>   * Scan the dispatch trace log and count up the stolen time.
>   * Should be called with interrupts disabled.
> diff --git a/arch/powerpc/platforms/chrp/time.c 
> b/arch/powerpc/platforms/chrp/time.c
> index acde7bbe0716..b94dfd5090d8 100644
> --- a/arch/powerpc/platforms/chrp/time.c
> +++ b/arch/powerpc/platforms/chrp/time.c
> @@ -30,8 +30,6 @@
>  
>  #include 
>  
> -extern spinlock_t rtc_lock;
> -
>  #define NVRAM_AS0  0x74
>  #define NVRAM_AS1  0x75
>  #define NVRAM_DATA 0x77
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings

2021-03-24 Thread Alexandre Belloni
On 24/03/2021 09:19:58+0100, Geert Uytterhoeven wrote:
> Hi Alexandre,
> 
> On Tue, Mar 23, 2021 at 11:18 PM Alexandre Belloni
>  wrote:
> > On 23/03/2021 05:12:57-0400, He Ying wrote:
> > > We found these warnings in arch/powerpc/kernel/time.c as follows:
> > > warning: symbol 'decrementer_max' was not declared. Should it be static?
> > > warning: symbol 'rtc_lock' was not declared. Should it be static?
> > > warning: symbol 'dtl_consumer' was not declared. Should it be static?
> > >
> > > Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
> > > Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
> > > avoid the conflict with the variable in powerpc asm/time.h.
> > > Move 'dtl_consumer' definition behind "include " because it
> > > is declared there.
> > >
> > > Reported-by: Hulk Robot 
> > > Signed-off-by: He Ying 
> > > ---
> > > v2:
> > > - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, 
> > > declare
> > >   rtc_lock in powerpc asm/time.h.
> > >
> >
> > V1 was actually the correct thing to do. rtc_lock is there exactly
> > because chrp and maple are using mc146818 compatible RTCs. This is then
> > useful because then drivers/char/nvram.c is enabled. The proper fix
> > would be to scrap all of that and use rtc-cmos for those platforms as
> > this drives the RTC properly and exposes the NVRAM for the mc146818.
> >
> > Or at least, if there are no users for the char/nvram driver on those
> > two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
> > more likely rename the symbol as it seems to be abused by both chrp and
> > powermac.
> 
> IIRC, on CHRP LongTrail, NVRAM was inherited from CHRP's Mac ancestry,
> not from CHRP's PC ancestry, and thus NVRAM is not the one in the
> mc146818-compatible RTC.
> 
> http://users.telenet.be/geertu/Linux/PPC/DeviceTree.html confirms that,
> showing that nvram is a different device node than rtc.
> 

Yes, what I missed was the ifdefery in drivers/char/nvram.c that makes
it a completely different driver on both platforms. I tend to forget
about that as reading this driver is not a pleasant experience. I would
really like to get rid of the x86 part which would in turn allow to
remove the global rtc_lock spinlock on all architectures.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings

2021-03-23 Thread Alexandre Belloni
On 23/03/2021 23:18:17+0100, Alexandre Belloni wrote:
> Hello,
> 
> On 23/03/2021 05:12:57-0400, He Ying wrote:
> > We found these warnings in arch/powerpc/kernel/time.c as follows:
> > warning: symbol 'decrementer_max' was not declared. Should it be static?
> > warning: symbol 'rtc_lock' was not declared. Should it be static?
> > warning: symbol 'dtl_consumer' was not declared. Should it be static?
> > 
> > Declare 'decrementer_max' and 'rtc_lock' in powerpc asm/time.h.
> > Rename 'rtc_lock' in drviers/rtc/rtc-vr41xx.c to 'vr41xx_rtc_lock' to
> > avoid the conflict with the variable in powerpc asm/time.h.
> > Move 'dtl_consumer' definition behind "include " because it
> > is declared there.
> > 
> > Reported-by: Hulk Robot 
> > Signed-off-by: He Ying 
> > ---
> > v2:
> > - Instead of including linux/mc146818rtc.h in powerpc kernel/time.c, declare
> >   rtc_lock in powerpc asm/time.h.
> > 
> 
> V1 was actually the correct thing to do. rtc_lock is there exactly
> because chrp and maple are using mc146818 compatible RTCs. This is then
> useful because then drivers/char/nvram.c is enabled. The proper fix
> would be to scrap all of that and use rtc-cmos for those platforms as
> this drives the RTC properly and exposes the NVRAM for the mc146818.
> 
> Or at least, if there are no users for the char/nvram driver on those
> two platforms, remove the spinlock and stop enabling CONFIG_NVRAM or
> more likely rename the symbol as it seems to be abused by both chrp and
> powermac.
> 

Ok so rtc_lock is not even used by the char/nvram.c driver as it is
completely compiled out.

I guess it is fine having it move to the individual platform as looking
very quickly at the Kconfig, it is not possible to select both
simultaneously. Tentative patch:

8<-
>From dfa59b6f44fdfdefafffa7666aec89e62bbd5c80 Mon Sep 17 00:00:00 2001
From: Alexandre Belloni 
Date: Wed, 24 Mar 2021 00:00:03 +0100
Subject: [PATCH] powerpc: move rtc_lock to specific platforms

Signed-off-by: Alexandre Belloni 
---
 arch/powerpc/kernel/time.c  | 3 ---
 arch/powerpc/platforms/chrp/time.c  | 2 +-
 arch/powerpc/platforms/maple/time.c | 2 ++
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 67feb3524460..d3bb189ea7f4 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -123,9 +123,6 @@ EXPORT_SYMBOL(tb_ticks_per_usec);
 unsigned long tb_ticks_per_sec;
 EXPORT_SYMBOL(tb_ticks_per_sec);   /* for cputime_t conversions */
 
-DEFINE_SPINLOCK(rtc_lock);
-EXPORT_SYMBOL_GPL(rtc_lock);
-
 static u64 tb_to_ns_scale __read_mostly;
 static unsigned tb_to_ns_shift __read_mostly;
 static u64 boot_tb __read_mostly;
diff --git a/arch/powerpc/platforms/chrp/time.c 
b/arch/powerpc/platforms/chrp/time.c
index acde7bbe0716..ea90c15f5edd 100644
--- a/arch/powerpc/platforms/chrp/time.c
+++ b/arch/powerpc/platforms/chrp/time.c
@@ -30,7 +30,7 @@
 
 #include 
 
-extern spinlock_t rtc_lock;
+DEFINE_SPINLOCK(rtc_lock);
 
 #define NVRAM_AS0  0x74
 #define NVRAM_AS1  0x75
diff --git a/arch/powerpc/platforms/maple/time.c 
b/arch/powerpc/platforms/maple/time.c
index 78209bb7629c..ddda02010d86 100644
--- a/arch/powerpc/platforms/maple/time.c
+++ b/arch/powerpc/platforms/maple/time.c
@@ -34,6 +34,8 @@
 #define DBG(x...)
 #endif
 
+DEFINE_SPINLOCK(rtc_lock);
+
 static int maple_rtc_addr;
 
 static int maple_clock_read(int addr)
-- 
2.25.1


> I'm not completely against the rename in vr41xxx but the fix for the
> warnings can and should be contained in arch/powerpc.
> 
> >  arch/powerpc/include/asm/time.h |  3 +++
> >  arch/powerpc/kernel/time.c  |  6 ++
> >  drivers/rtc/rtc-vr41xx.c| 22 +++---
> >  3 files changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/time.h 
> > b/arch/powerpc/include/asm/time.h
> > index 8dd3cdb25338..64a3ef0b4270 100644
> > --- a/arch/powerpc/include/asm/time.h
> > +++ b/arch/powerpc/include/asm/time.h
> > @@ -12,6 +12,7 @@
> >  #ifdef __KERNEL__
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include 
> >  #include 
> > @@ -22,6 +23,8 @@ extern unsigned long tb_ticks_per_jiffy;
> >  extern unsigned long tb_ticks_per_usec;
> >  extern unsigned long tb_ticks_per_sec;
> >  extern struct clock_event_device decrementer_clockevent;
> > +extern u64 decrementer_max;
> > +extern spinlock_t rtc_lock;
> >  
> >  
> >  extern void generic_calibrate_decr(void);
> > diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> > index b67d93a609a2..60b6ac7d3685 100644
> > --- a/arch/powerpc/kernel/time.c
> > +++ b/arch/

Re: [PATCH v2 -next] powerpc: kernel/time.c - cleanup warnings

2021-03-23 Thread Alexandre Belloni
ime64_to_tm((high << 17) | (mid << 1) | (low >> 15), time);
>  
> @@ -159,7 +159,7 @@ static int vr41xx_rtc_set_alarm(struct device *dev, 
> struct rtc_wkalrm *wkalrm)
>  
>   alarm_sec = rtc_tm_to_time64(>time);
>  
> - spin_lock_irq(_lock);
> + spin_lock_irq(_rtc_lock);
>  
>   if (alarm_enabled)
>   disable_irq(aie_irq);
> @@ -173,7 +173,7 @@ static int vr41xx_rtc_set_alarm(struct device *dev, 
> struct rtc_wkalrm *wkalrm)
>  
>   alarm_enabled = wkalrm->enabled;
>  
> - spin_unlock_irq(_lock);
> + spin_unlock_irq(_rtc_lock);
>  
>   return 0;
>  }
> @@ -202,7 +202,7 @@ static int vr41xx_rtc_ioctl(struct device *dev, unsigned 
> int cmd, unsigned long
>  
>  static int vr41xx_rtc_alarm_irq_enable(struct device *dev, unsigned int 
> enabled)
>  {
> - spin_lock_irq(_lock);
> + spin_lock_irq(_rtc_lock);
>   if (enabled) {
>   if (!alarm_enabled) {
>   enable_irq(aie_irq);
> @@ -214,7 +214,7 @@ static int vr41xx_rtc_alarm_irq_enable(struct device 
> *dev, unsigned int enabled)
>   alarm_enabled = 0;
>   }
>   }
> - spin_unlock_irq(_lock);
> + spin_unlock_irq(_rtc_lock);
>   return 0;
>  }
>  
> @@ -296,7 +296,7 @@ static int rtc_probe(struct platform_device *pdev)
>   rtc->range_max = (1ULL << 33) - 1;
>   rtc->max_user_freq = MAX_PERIODIC_RATE;
>  
> - spin_lock_irq(_lock);
> + spin_lock_irq(_rtc_lock);
>  
>   rtc1_write(ECMPLREG, 0);
>   rtc1_write(ECMPMREG, 0);
> @@ -304,7 +304,7 @@ static int rtc_probe(struct platform_device *pdev)
>   rtc1_write(RTCL1LREG, 0);
>   rtc1_write(RTCL1HREG, 0);
>  
> - spin_unlock_irq(_lock);
> + spin_unlock_irq(_rtc_lock);
>  
>   aie_irq = platform_get_irq(pdev, 0);
>   if (aie_irq <= 0) {
> -- 
> 2.17.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] dt-bindings: i3c: Fix silvaco,i3c-master-v1 compatible string

2021-03-23 Thread Alexandre Belloni
On Thu, 11 Mar 2021 16:40:56 -0700, Rob Herring wrote:
> The example for the Silvaco I3C master doesn't match the schema's
> compatible string. Fix it.

Applied, thanks!

[1/1] dt-bindings: i3c: Fix silvaco,i3c-master-v1 compatible string
  commit: e43d5c7c3c3459b428431754672052503c5db9c8

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH v5] rtc: rx6110: add ACPI bindings to I2C

2021-03-23 Thread Alexandre Belloni
On 17/03/2021 10:32:39+0100, Claudius Heine wrote:
> Hi Andy,
> 
> On 2021-03-17 10:28, Andy Shevchenko wrote:
> > On Wed, Mar 17, 2021 at 9:56 AM Claudius Heine  wrote:
> > > 
> > > From: Johannes Hahn 
> > > 
> > > This allows the RX6110 driver to be automatically assigned to the right
> > > device on the I2C bus.
> > 
> > Thanks for an update!
> > 
> > > Signed-off-by: Johannes Hahn 
> > > Co-developed-by: Claudius Heine 
> > > Signed-off-by: Claudius Heine 
> > > Reviewed-by: Andy Shevchenko 
> > 
> > > Reported-by: kernel test robot 
> > 
> > This is usually for patches that do fix found problems, here it's a
> > completely new item and the report was done in the middle of the
> > development. That said, you may give credit to LKP by just mentioning
> > it in the comments section (after the cutter '---' line). I'll leave
> > this to Alexandre and Alessandro to decide if you need a resend or
> > they may remove it when applying. (In my opinion resend is not needed
> > right now)
> Ok. Thanks a lot for your reviews and patience!
> 

I removed it when applying. Thanks for the work and the reviews!


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v5] rtc: rx6110: add ACPI bindings to I2C

2021-03-23 Thread Alexandre Belloni
On Wed, 17 Mar 2021 08:52:27 +0100, Claudius Heine wrote:
> This allows the RX6110 driver to be automatically assigned to the right
> device on the I2C bus.

Applied, thanks!

[1/1] rtc: rx6110: add ACPI bindings to I2C
  commit: 8d69f62fddf6c1a8c7745120c4d6aab9322b001a

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH -next] phy: sparx5_serdes: Fix return value check in sparx5_serdes_probe()

2021-03-18 Thread Alexandre Belloni
Hello,

On 18/03/2021 13:56:47+, 'w00385741 wrote:
> From: Wei Yongjun 
> 
> In case of error, the function devm_ioremap() returns NULL
> pointer not ERR_PTR(). The IS_ERR() test in the return value
> check should be replaced with NULL test.
> 
> Fixes: 2ff8a1eeb5aa ("phy: Add Sparx5 ethernet serdes PHY driver")
> Reported-by: Hulk Robot 
> Signed-off-by: Wei Yongjun 
> ---
>  drivers/phy/microchip/sparx5_serdes.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/microchip/sparx5_serdes.c 
> b/drivers/phy/microchip/sparx5_serdes.c
> index 06bcf0c166cf..dd854d825000 100644
> --- a/drivers/phy/microchip/sparx5_serdes.c
> +++ b/drivers/phy/microchip/sparx5_serdes.c
> @@ -2438,10 +2438,10 @@ static int sparx5_serdes_probe(struct platform_device 
> *pdev)
>  
>   iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   iomem = devm_ioremap(priv->dev, iores->start, iores->end - iores->start 
> + 1);
> - if (IS_ERR(iomem)) {
> + if (!iomem) {
>   dev_err(priv->dev, "Unable to get serdes registers: %s\n",
>   iores->name);
> - return PTR_ERR(iomem);
> + return -ENOMEM;
>   }

A better fix would use devm_platform_ioremap_resource and get rid of the
error messages

>   for (idx = 0; idx < ARRAY_SIZE(sparx5_serdes_iomap); idx++) {
>   struct sparx5_serdes_io_resource *iomap = 
> _serdes_iomap[idx];
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] MIPS: generic: use true and false for bool variable

2021-03-17 Thread Alexandre Belloni
On 15/03/2021 15:15:43+0800, Yang Li wrote:
> fixed the following coccicheck:
> ./arch/mips/generic/board-ocelot.c:29:9-10: WARNING: return of 0/1 in
> function 'ocelot_detect' with return type bool
> 

Can you elaborate why this is an issue and what exactly is fixed?
Even the original coccinelle script submission doesn't give any good
reason.

bool in Linux is _Bool and, as per C99, it holds either 0 or 1, not true
or false.

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] dt-bindings: Clean-up undocumented compatible strings

2021-03-17 Thread Alexandre Belloni
On 16/03/2021 13:49:18-0600, Rob Herring wrote:
> Adding checks for undocumented compatible strings reveals a bunch of
> warnings in the DT binding examples. Fix the cases which are typos, just
> a mismatch between the schema and the example, or aren't documented at all.
> In a couple of cases, fixing the compatible revealed some schema errors
> which are fixed.
> 
> There's a bunch of others remaining after this which have bindings, but
> those aren't converted to schema yet.
> 
> Cc: Stephen Boyd 
> Cc: Maxime Ripard 
> Cc: Thierry Reding 
> Cc: Sam Ravnborg 
> Cc: Vinod Koul 
> Cc: Alexandre Belloni 
> Cc: Jonathan Cameron 
> Cc: Pavel Machek 
> Cc: Kishon Vijay Abraham I 
> Cc: Sebastian Reichel 
> Cc: Mark Brown 
> Cc: Greg Kroah-Hartman 
> Cc: linux-...@vger.kernel.org
> Cc: dmaeng...@vger.kernel.org
> Cc: linux-...@lists.infradead.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-l...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: linux-ser...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Rob Herring 

Acked-by: Alexandre Belloni 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] rtc: ds1307: set uie_unsupported if no interrupt is available

2021-03-16 Thread Alexandre Belloni
On 16/03/2021 13:12:08+0100, Lukasz Stelmach wrote:
> It was <2021-03-15 pon 23:01>, when Alexandre Belloni wrote:
> > Hello,
> >
> > On 05/03/2021 18:44:11+0100, Łukasz Stelmach wrote:
> >> For an RTC without an IRQ assigned rtc_update_irq_enable() should
> >> return -EINVAL.  It will, when uie_unsupported is set.
> >> 
> >
> > I'm surprised this is an issue because the current code seems to cover
> > all cases:
> >
> >  - no irq and not wakeup-source => set_alarm should fail
> >  - no irq and wakeup-source => uie_unsupported is set
> >  - irq => UIE should work
> >
> > Can you elaborate on your failing use case?
> 
> I've got ds3231 which supports alarms[1] but is not connected to any
> interrupt line. Hence, client->irq is 0 as well as want_irq[2]. There
> is also no other indirect connection, so I don't set wakeup-source
> property and ds1307_can_wakeup_device remains[3] false. Under these
> conditions
> 
> want_irq = 0
> ds1307_can_wakeup_device = false
> 
> uie_unsupported remains[4] false. And this is the problem.
> 
> hwclock(8) when setting system clock from rtc (--hctosys) calls
> synchronize_to_clock_tick_rtc()[5]. There goes
> 
> ioctl(rtc_fd, RTC_UIE_ON, 0);
> 
> which leads us to
> 
> rtc_update_irq_enable(rtc, 1);
> 
> and finally here [6]
> 
> if (rtc->uie_unsupported) {
> err = -EINVAL;
> goto out;
> }
> 
> and we keep going (uie_unsupported = 0). All the following operations
> succeed because chip supports alarms.
> 

But then, HAS_ALARM is not set and ds1337_set_alarm should fail which
makes rtc_timer_enqueue return an error. I admit this whole part is a
mess, I'm just trying to understand how you can hit that.

> We go back to hwclock(8) and we start waiting[7] for the update from
> interrupt which never arrives instead of calling
> busywiat_for_rtc_clock_tick()[8] (mind the invalid indentation) because
> of EINVAL returned from ioctl() (conf. [6])
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/rtc-ds1307.c?h=v5.11#n1032
> [2] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/rtc-ds1307.c?h=v5.11#n1779
> [3] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/rtc-ds1307.c?h=v5.11#n1802
> [4] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/rtc-ds1307.c?h=v5.11#n1977
> [5] 
> https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/sys-utils/hwclock-rtc.c?h=v2.36.2#n252
> [6] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/rtc/interface.c?h=v5.11#n564
> [7] 
> https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/sys-utils/hwclock-rtc.c?h=v2.36.2#n283
> [8] 
> https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/sys-utils/hwclock-rtc.c?h=v2.36.2#n297
> 
> >> Signed-off-by: Łukasz Stelmach 
> >> ---
> >>  drivers/rtc/rtc-ds1307.c | 14 +++---
> >>  1 file changed, 7 insertions(+), 7 deletions(-)
> >> 
> >> diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
> >> index cd8e438bc9c4..b08a9736fa77 100644
> >> --- a/drivers/rtc/rtc-ds1307.c
> >> +++ b/drivers/rtc/rtc-ds1307.c
> >> @@ -1973,13 +1973,6 @@ static int ds1307_probe(struct i2c_client *client,
> >>if (IS_ERR(ds1307->rtc))
> >>return PTR_ERR(ds1307->rtc);
> >>  
> >> -  if (ds1307_can_wakeup_device && !want_irq) {
> >> -  dev_info(ds1307->dev,
> >> -   "'wakeup-source' is set, request for an IRQ is 
> >> disabled!\n");
> >> -  /* We cannot support UIE mode if we do not have an IRQ line */
> >> -  ds1307->rtc->uie_unsupported = 1;
> >> -  }
> >> -
> >>if (want_irq) {
> >>err = devm_request_threaded_irq(ds1307->dev, client->irq, NULL,
> >>chip->irq_handler ?: ds1307_irq,
> >> @@ -1993,6 +1986,13 @@ static int ds1307_probe(struct i2c_client *client,
> >>} else {
> >>dev_dbg(ds1307->dev, "got IRQ %d\n", client->irq);
> >>}
> >> +  } else {
> >> +  if (ds1307_can_wakeup_device)
> >> +  dev_info(ds1307->dev,
> >> +   "'wakeup-source' is set, request for an IRQ is 
> >> disabled!\n");
> >> +
> >
> > Honestly, just drop this message, it should have been removed by 
> > 82e2d43f6315
> >
> >
> 
> Done.
> 
> >> +  /* We cannot support UIE mode if we do not have an IRQ line */
> >> +  ds1307->rtc->uie_unsupported = 1;
> >>}
> >>  
> >>ds1307->rtc->ops = chip->rtc_ops ?: _rtc_ops;
> >> -- 
> >> 2.26.2
> >> 
> 
> -- 
> Łukasz Stelmach
> Samsung R Institute Poland
> Samsung Electronics



-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] rtc: rv3028: correct weekday register usage

2021-03-15 Thread Alexandre Belloni
On Tue, 9 Mar 2021 14:47:19 +0100, Heiko Schocher wrote:
> The datasheet for the rv3028 says the weekday has exact 3 bits
> and in chapter 3.4.0 for the "3h–Weekday" register it says:
> """
> This register holds the current day of the week. Each value represents
> one weekday that is assigned by the user. Values will range from 0 to 6
> The weekday counter is simply a 3-bit counter which counts up to 6
> and then resets to 0.
> """
> 
> [...]

Applied, thanks!

[1/1] rtc: rv3028: correct weekday register usage
  commit: 6e00b6d0083ea5f529b057e87c0236747871b6a8

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH] rtc: cmos: Disable irq around direct invocation of cmos_interrupt()

2021-03-15 Thread Alexandre Belloni
On 05/03/2021 15:27:52+0200, Ville Syrjälä wrote:
> > which breaks S3-resume on fi-kbl-soraka presumably as that's slow enough
> > to trigger the alarm during the suspend.
> > 
> > Fixes: 6950d046eb6e ("rtc: cmos: Replace spin_lock_irqsave with spin_lock 
> > in hard IRQ")
> 
> Sigh. I wish people would at least try to check the code/history
> before doing these blind "cleanups" :(
>

Sorry I didn't catch that when applying. I'm usually more wary when
getting those kind of cleanups.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH] rtc: cmos: Disable irq around direct invocation of cmos_interrupt()

2021-03-15 Thread Alexandre Belloni
On Fri, 5 Mar 2021 12:21:40 +, Chris Wilson wrote:
> As previously noted in commit 66e4f4a9cc38 ("rtc: cmos: Use
> spin_lock_irqsave() in cmos_interrupt()"):
> 
> <4>[  254.192378] WARNING: inconsistent lock state
> <4>[  254.192384] 5.12.0-rc1-CI-CI_DRM_9834+ #1 Not tainted
> <4>[  254.192396] 
> <4>[  254.192400] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
> <4>[  254.192409] rtcwake/5309 [HC0[0]:SC0[0]:HE1:SE1] takes:
> <4>[  254.192429] 8263c5f8 (rtc_lock){?...}-{2:2}, at: 
> cmos_interrupt+0x18/0x100
> <4>[  254.192481] {IN-HARDIRQ-W} state was registered at:
> <4>[  254.192488]   lock_acquire+0xd1/0x3d0
> <4>[  254.192504]   _raw_spin_lock+0x2a/0x40
> <4>[  254.192519]   cmos_interrupt+0x18/0x100
> <4>[  254.192536]   rtc_handler+0x1f/0xc0
> <4>[  254.192553]   acpi_ev_fixed_event_detect+0x109/0x13c
> <4>[  254.192574]   acpi_ev_sci_xrupt_handler+0xb/0x28
> <4>[  254.192596]   acpi_irq+0x13/0x30
> <4>[  254.192620]   __handle_irq_event_percpu+0x43/0x2c0
> <4>[  254.192641]   handle_irq_event_percpu+0x2b/0x70
> <4>[  254.192661]   handle_irq_event+0x2f/0x50
> <4>[  254.192680]   handle_fasteoi_irq+0x9e/0x150
> <4>[  254.192693]   __common_interrupt+0x76/0x140
> <4>[  254.192715]   common_interrupt+0x96/0xc0
> <4>[  254.192732]   asm_common_interrupt+0x1e/0x40
> <4>[  254.192750]   _raw_spin_unlock_irqrestore+0x38/0x60
> <4>[  254.192767]   resume_irqs+0xba/0xf0
> <4>[  254.192786]   dpm_resume_noirq+0x245/0x3d0
> <4>[  254.192811]   suspend_devices_and_enter+0x230/0xaa0
> <4>[  254.192835]   pm_suspend.cold.8+0x301/0x34a
> <4>[  254.192859]   state_store+0x7b/0xe0
> <4>[  254.192879]   kernfs_fop_write_iter+0x11d/0x1c0
> <4>[  254.192899]   new_sync_write+0x11d/0x1b0
> <4>[  254.192916]   vfs_write+0x265/0x390
> <4>[  254.192933]   ksys_write+0x5a/0xd0
> <4>[  254.192949]   do_syscall_64+0x33/0x80
> <4>[  254.192965]   entry_SYSCALL_64_after_hwframe+0x44/0xae
> <4>[  254.192986] irq event stamp: 43775
> <4>[  254.192994] hardirqs last  enabled at (43775): [] 
> asm_sysvec_apic_timer_interrupt+0x12/0x20
> <4>[  254.193023] hardirqs last disabled at (43774): [] 
> sysvec_apic_timer_interrupt+0xa/0xb0
> <4>[  254.193049] softirqs last  enabled at (42548): [] 
> __do_softirq+0x342/0x48e
> <4>[  254.193074] softirqs last disabled at (42543): [] 
> irq_exit_rcu+0xad/0xd0
> <4>[  254.193101]
>   other info that might help us debug this:
> <4>[  254.193107]  Possible unsafe locking scenario:
> 
> [...]

Applied, thanks!

[1/1] rtc: cmos: Disable irq around direct invocation of cmos_interrupt()
  commit: bd5aa93d615cac77d991c448b986761e7a8d

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH] rtc: tps65910: include linux/property.h

2021-03-15 Thread Alexandre Belloni
On Thu, 25 Feb 2021 14:42:04 +0100, Arnd Bergmann wrote:
> The added device_property_present() call causes a build
> failure in some configurations because of the missing header:
> 
> drivers/rtc/rtc-tps65910.c:422:7: error: implicit declaration of function 
> 'device_property_present' [-Werror,-Wimplicit-function-declaration]

Applied, thanks!

[1/1] rtc: tps65910: include linux/property.h
  commit: 936d3685e62436a378f02b8b74759b054d4aeca1

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH v9 2/4] pinctrl: pinmux: Add pinmux-select debugfs file

2021-03-13 Thread Alexandre Belloni
On 12/03/2021 14:57:54+0100, Enrico Weigelt, metux IT consult wrote:
> On 02.03.21 06:30, Drew Fustini wrote:
> 
> Hi folks,
> 
> > Add "pinmux-select" to debugfs which will activate a pin function for a
> > given pin group:
> > 
> >echo "" > pinmux-select
> > 
> > The write operation pinmux_select() handles this by checking that the
> > names map to valid selectors and then calling ops->set_mux().
> 
> I've already been playing with similar idea, but for external muxes.
> For example, some boards have multiple SIM slots that can be switched
> via some gpio pin.
> 
> Not sure whether traditional pinmux would be a good match for that.
> 

If you want to be able to use both, then I guess gpio-mux is what you
are looking for. Obviously, it will also require support in the bus
core. On what bus are those SIMs? (I guess the answer will be UART and
then unfortunately UARTs are not represented as busses).


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: rtc: rtc-m48t59: rtc-m48t59.0: IRQ index 0 not found

2021-03-11 Thread Alexandre Belloni
Hello,

On 10/03/2021 10:53:34+0100, Corentin Labbe wrote:
> Hello
> 
> On my SPARC sunblade 100, I got this:
> [   13.613727] rtc-m48t59 rtc-m48t59.0: IRQ index 0 not found
> [   13.805777] rtc-m48t59 rtc-m48t59.0: registered as rtc0
> [   14.385092] rtc-m48t59 rtc-m48t59.0: setting system clock to 
> 2021-03-01T05:34:33 UTC (1614576873)
> 
> The IRQ index 0 message is found after 5.5
> 
> Testing rtc via hwclock give:
> hwclock: ioctl(3, RTC_UIE_ON, 0) to /dev/rtc0 failed: Input/output error
> But this hwclock behavior is present also on earlier kernel (tested 4.9.260, 
> 4.19.179 and 4.14.224).
> 
> Does this Input/output error is normal ? (I think no)

This is due to 7723f4c5ecdb8d832f049f8483beb0d1081cedf6

Can you try that?

>From 55cc33fab5ac9f7e2a97aa7c564e8b35355886d5 Mon Sep 17 00:00:00 2001
From: Alexandre Belloni 
Date: Thu, 11 Mar 2021 09:48:09 +0100
Subject: [PATCH] rtc: m48t59: use platform_get_irq_optional

The IRQ is optional, avoid the error message by using
platform_get_irq_optional.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-m48t59.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index 1d2e99a70fce..f0f6b9b6daec 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -421,7 +421,7 @@ static int m48t59_rtc_probe(struct platform_device *pdev)
/* Try to get irq number. We also can work in
 * the mode without IRQ.
 */
-   m48t59->irq = platform_get_irq(pdev, 0);
+   m48t59->irq = platform_get_irq_optional(pdev, 0);
if (m48t59->irq <= 0)
        m48t59->irq = NO_IRQ;
 
-- 
2.29.2

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [GIT PULL] Immutable branch between MFD, PWM and RTC due for the v5.13 merge window

2021-03-10 Thread Alexandre Belloni
On 10/03/2021 12:39:59+0100, Uwe Kleine-König wrote:
> Hello Lee,
> 
> On Tue, Mar 09, 2021 at 08:05:20PM +, Lee Jones wrote:
> > On Mon, 01 Mar 2021, Lee Jones wrote:
> > 
> > > The following changes since commit 
> > > fe07bfda2fb9cdef8a4d4008a409bb02f35f1bd8:
> > > 
> > >   Linux 5.12-rc1 (2021-02-28 16:05:19 -0800)
> > > 
> > > are available in the Git repository at:
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git 
> > > ib-mfd-pwm-rtc-v5.13
> > > 
> > > for you to fetch changes up to 80629611215d1c5d52ed3cf723fd6d24a5872504:
> > > 
> > >   MAINTAINERS: Add entry for Netronix embedded controller (2021-03-01 
> > > 10:26:17 +)
> > > 
> > > 
> > > Immutable branch between MFD, PWM and RTC due for the v5.13 merge window
> > > 
> > > 
> > > [...]
> > 
> > FYI, if anyone has pulled this, they should probably rebase it onto
> > v5.12-rc2 and delete the v5.12-rc1 tag from their tree:
> > 
> >   https://lwn.net/Articles/848431/
> 
> I'm not directly affected, but I wonder: The idea of an immutable branch
> is that the same history gets included in different trees. If now each
> maintainer rebases individually the result isn't the same
> history any more in each tree which somewhat defeats the idea of using
> immutable branches.
> 
> IMHO there are two ways forward: Either someone (Lee again?) creates a
> new pull request for this series rebased on -rc2; or we accept that
> these few patches are based on -rc1. For the latter it would be
> beneficial to merge the tag into a tree that is already based on -rc2.
> 

The solution is simply for the maintainers merging the immutable branch
to do that in a branch based on -rc2. Eg. I've rebased rtc-next on -rc2
(fast forward, I didn't have any patch). I can now merge this branch if
necessary, problem solved. If you can't rebased, nothing prevents you
from merging -rc2 in any branch.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH RESEND][next] i3c: master: cdns: Fix fall-through warnings for Clang

2021-03-05 Thread Alexandre Belloni
On 05/03/2021 04:01:23-0600, Gustavo A. R. Silva wrote:
> In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning
> by explicitly adding a break statement instead of letting the code fall
> through to the next case.
> 

I though Clang would be fixed to stop warning in that case, can't you
push a patch there ?

> Link: https://github.com/KSPP/linux/issues/115
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/i3c/master/i3c-master-cdns.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i3c/master/i3c-master-cdns.c 
> b/drivers/i3c/master/i3c-master-cdns.c
> index 3f2226928fe0..5b37ffe5ad5b 100644
> --- a/drivers/i3c/master/i3c-master-cdns.c
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -1379,6 +1379,8 @@ static void cnds_i3c_master_demux_ibis(struct 
> cdns_i3c_master *master)
>  
>   case IBIR_TYPE_MR:
>   WARN_ON(IBIR_XFER_BYTES(ibir) || (ibir & IBIR_ERROR));
> + break;
> +
>   default:
>       break;
>   }
> -- 
> 2.27.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH][next] i3c: master: svc: remove redundant assignment to cmd->read_len

2021-03-04 Thread Alexandre Belloni
On Wed, 24 Feb 2021 15:13:49 +, Colin King wrote:
> The assignment of xfer_len to cmd->read_len appears to be redundant
> as the next statement re-assigns the value 0 to it.  Clean up the
> code by removing the redundant first assignment.

Applied, thanks!

[1/1] i3c: master: svc: remove redundant assignment to cmd->read_len
  commit: 437f5e2af73081ec08ec5d73d82c650377a4bb17

Best regards,
-- 
Alexandre Belloni 


Re: [RESEND 1/1] arch: arm: mach-at91: pm: Move prototypes to mutually included header

2021-03-03 Thread Alexandre Belloni
On 03/03/2021 12:41:49+, Lee Jones wrote:
> Both the caller and the supplier's source file should have access to
> the include file containing the prototypes.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/pinctrl/pinctrl-at91.c:1637:6: warning: no previous prototype for 
> ‘at91_pinctrl_gpio_suspend’ [-Wmissing-prototypes]
>  1637 | void at91_pinctrl_gpio_suspend(void)
>  | ^
>  drivers/pinctrl/pinctrl-at91.c:1661:6: warning: no previous prototype for 
> ‘at91_pinctrl_gpio_resume’ [-Wmissing-prototypes]
>  1661 | void at91_pinctrl_gpio_resume(void)
>  | ^~~~
> 
> Cc: Russell King 
> Cc: Nicolas Ferre 
> Cc: Alexandre Belloni 
> Cc: Ludovic Desroches 
> Signed-off-by: Lee Jones 

I'm pretty sure you had my ack on v3 ;)

Acked-by: Alexandre Belloni 

or again, alternatively, I can apply it with Linus' ack

> ---
>  arch/arm/mach-at91/pm.c| 19 ---
>  drivers/pinctrl/pinctrl-at91.c |  2 ++
>  include/soc/at91/pm.h  | 16 
>  3 files changed, 26 insertions(+), 11 deletions(-)
>  create mode 100644 include/soc/at91/pm.h
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 120f9aa6fff32..90dcdfe3b3d0d 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>  
> +#include 
> +
>  #include 
>  #include 
>  #include 
> @@ -25,17 +27,6 @@
>  #include "generic.h"
>  #include "pm.h"
>  
> -/*
> - * FIXME: this is needed to communicate between the pinctrl driver and
> - * the PM implementation in the machine. Possibly part of the PM
> - * implementation should be moved down into the pinctrl driver and get
> - * called as part of the generic suspend/resume path.
> - */
> -#ifdef CONFIG_PINCTRL_AT91
> -extern void at91_pinctrl_gpio_suspend(void);
> -extern void at91_pinctrl_gpio_resume(void);
> -#endif
> -
>  struct at91_soc_pm {
>   int (*config_shdwc_ws)(void __iomem *shdwc, u32 *mode, u32 *polarity);
>   int (*config_pmc_ws)(void __iomem *pmc, u32 mode, u32 polarity);
> @@ -326,6 +317,12 @@ static void at91_pm_suspend(suspend_state_t state)
>  static int at91_pm_enter(suspend_state_t state)
>  {
>  #ifdef CONFIG_PINCTRL_AT91
> + /*
> +  * FIXME: this is needed to communicate between the pinctrl driver and
> +  * the PM implementation in the machine. Possibly part of the PM
> +  * implementation should be moved down into the pinctrl driver and get
> +  * called as part of the generic suspend/resume path.
> +  */
>   at91_pinctrl_gpio_suspend();
>  #endif
>  
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index 8003d1bd16953..fc61aaec34cc9 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -23,6 +23,8 @@
>  /* Since we request GPIOs from ourself */
>  #include 
>  
> +#include 
> +
>  #include "pinctrl-at91.h"
>  #include "core.h"
>  
> diff --git a/include/soc/at91/pm.h b/include/soc/at91/pm.h
> new file mode 100644
> index 0..7a41e53a3ffa3
> --- /dev/null
> +++ b/include/soc/at91/pm.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Atmel Power Management
> + *
> + * Copyright (C) 2020 Atmel
> + *
> + * Author: Lee Jones 
> + */
> +
> +#ifndef __SOC_ATMEL_PM_H
> +#define __SOC_ATMEL_PM_H
> +
> +void at91_pinctrl_gpio_suspend(void);
> +void at91_pinctrl_gpio_resume(void);
> +
> +#endif /* __SOC_ATMEL_PM_H */
> -- 
> 2.27.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v7 3/3] arm64: dts: reset: add microchip sparx5 switch reset driver

2021-03-03 Thread Alexandre Belloni
On 03/03/2021 09:11:58+0100, Steen Hegelund wrote:
> This provides reset driver support for the Microchip Sparx5 PCB134 and
> PCB135 reference boards.
> 
> Signed-off-by: Steen Hegelund 
Reviewed-by: Alexandre Belloni 

> ---
>  arch/arm64/boot/dts/microchip/sparx5.dtsi | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/microchip/sparx5.dtsi 
> b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> index 380281f312d8..dc3ada5cf9fc 100644
> --- a/arch/arm64/boot/dts/microchip/sparx5.dtsi
> +++ b/arch/arm64/boot/dts/microchip/sparx5.dtsi
> @@ -132,9 +132,12 @@ mux: mux-controller {
>   };
>   };
>  
> - reset@611010008 {
> - compatible = "microchip,sparx5-chip-reset";
> + reset: reset-controller@611010008 {
> + compatible = "microchip,sparx5-switch-reset";
>   reg = <0x6 0x11010008 0x4>;
> + reg-names = "gcb";
> + #reset-cells = <1>;
> +     cpu-syscon = <_ctrl>;
>   };
>  
>   uart0: serial@60010 {
> -- 
> 2.30.1
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v7 2/3] reset: mchp: sparx5: add switch reset driver

2021-03-03 Thread Alexandre Belloni
On 03/03/2021 09:11:57+0100, Steen Hegelund wrote:
> The Sparx5 Switch SoC has a number of components that can be reset
> indiviually, but at least the Switch Core needs to be in a well defined
> state at power on, when any of the Sparx5 drivers starts to access the
> Switch Core, this reset driver is available.
> 
> The reset driver is loaded early via the postcore_initcall interface, and
> will then be available for the other Sparx5 drivers (SGPIO, SwitchDev etc)
> that are loaded next, and the first of them to be loaded can perform the
> one-time Switch Core reset that is needed.
> 
> The driver has protection so that the system busses, DDR controller, PCI-E
> and ARM A53 CPU and a few other subsystems are not touched by the reset.
> 
> Signed-off-by: Steen Hegelund 
Reviewed-by: Alexandre Belloni 

> ---
>  drivers/reset/Kconfig  |   8 ++
>  drivers/reset/Makefile |   1 +
>  drivers/reset/reset-microchip-sparx5.c | 146 +
>  3 files changed, 155 insertions(+)
>  create mode 100644 drivers/reset/reset-microchip-sparx5.c
> 
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 4171c6f76385..c26798092ccf 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -111,6 +111,14 @@ config RESET_LPC18XX
>   help
> This enables the reset controller driver for NXP LPC18xx/43xx SoCs.
>  
> +config RESET_MCHP_SPARX5
> + bool "Microchip Sparx5 reset driver"
> + depends on HAS_IOMEM || COMPILE_TEST
> + default y if SPARX5_SWITCH
> + select MFD_SYSCON
> + help
> +   This driver supports switch core reset for the Microchip Sparx5 SoC.
> +
>  config RESET_MESON
>   tristate "Meson Reset Driver"
>   depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index 65a118a91b27..c1d6aa9b1b52 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_RESET_INTEL_GW) += reset-intel-gw.o
>  obj-$(CONFIG_RESET_K210) += reset-k210.o
>  obj-$(CONFIG_RESET_LANTIQ) += reset-lantiq.o
>  obj-$(CONFIG_RESET_LPC18XX) += reset-lpc18xx.o
> +obj-$(CONFIG_RESET_MCHP_SPARX5) += reset-microchip-sparx5.o
>  obj-$(CONFIG_RESET_MESON) += reset-meson.o
>  obj-$(CONFIG_RESET_MESON_AUDIO_ARB) += reset-meson-audio-arb.o
>  obj-$(CONFIG_RESET_NPCM) += reset-npcm.o
> diff --git a/drivers/reset/reset-microchip-sparx5.c 
> b/drivers/reset/reset-microchip-sparx5.c
> new file mode 100644
> index ..cff39a643a14
> --- /dev/null
> +++ b/drivers/reset/reset-microchip-sparx5.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Microchip Sparx5 Switch Reset driver
> + *
> + * Copyright (c) 2020 Microchip Technology Inc. and its subsidiaries.
> + *
> + * The Sparx5 Chip Register Model can be browsed at this location:
> + * https://github.com/microchip-ung/sparx-5_reginfo
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define PROTECT_REG0x84
> +#define PROTECT_BITBIT(10)
> +#define SOFT_RESET_REG 0x00
> +#define SOFT_RESET_BIT BIT(1)
> +
> +struct mchp_reset_context {
> + struct regmap *cpu_ctrl;
> + struct regmap *gcb_ctrl;
> + struct reset_controller_dev rcdev;
> +};
> +
> +static struct regmap_config sparx5_reset_regmap_config = {
> + .reg_bits   = 32,
> + .val_bits   = 32,
> + .reg_stride = 4,
> +};
> +
> +static int sparx5_switch_reset(struct reset_controller_dev *rcdev,
> +unsigned long id)
> +{
> + struct mchp_reset_context *ctx =
> + container_of(rcdev, struct mchp_reset_context, rcdev);
> + u32 val;
> +
> + /* Make sure the core is PROTECTED from reset */
> + regmap_update_bits(ctx->cpu_ctrl, PROTECT_REG, PROTECT_BIT, 
> PROTECT_BIT);
> +
> + /* Start soft reset */
> + regmap_write(ctx->gcb_ctrl, SOFT_RESET_REG, SOFT_RESET_BIT);
> +
> + /* Wait for soft reset done */
> + return regmap_read_poll_timeout(ctx->gcb_ctrl, SOFT_RESET_REG, val,
> + (val & SOFT_RESET_BIT) == 0,
> + 1, 100);
> +}
> +
> +static const struct reset_control_ops sparx5_reset_ops = {
> + .reset = sparx5_switch_reset,
> +};
> +
> +static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name,
> +   struct regmap **target)
> +{
> + struct device_node *syscon_np;
> + struct regmap *regmap;
> + int err;
> +
> + syscon_np = of_parse_phandle(pdev->dev.of_node,

Re: [PATCH v6 2/3] reset: mchp: sparx5: add switch reset driver

2021-02-25 Thread Alexandre Belloni
scon_np)
> + return -ENODEV;
> + regmap = syscon_node_to_regmap(syscon_np);
> + of_node_put(syscon_np);
> + if (IS_ERR(regmap)) {
> + err = PTR_ERR(regmap);
> + dev_err(>dev, "No '%s' map: %d\n", name, err);
> + return err;
> + }
> + *target = regmap;
> + return 0;
> +}
> +
> +static int mchp_sparx5_map_io(struct platform_device *pdev, char *name,
> +   struct regmap **target)
> +{
> + struct resource *res;
> + struct regmap *map;
> + void __iomem *mem;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
> + if (!res) {
> + dev_err(>dev, "No '%s' resource\n", name);
> + return -ENODEV;
> + }
> + mem = devm_ioremap(>dev, res->start, res->end - res->start + 1);
> + if (!mem) {
> + dev_err(>dev, "Could not map '%s' resource\n", name);
> + return -ENXIO;
> + }

Someone is going to tell you to use
devm_platform_get_and_ioremap_resource so it may as well be me ;)

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: watchdog: pcf2127: systemd fails on 5.11

2021-02-24 Thread Alexandre Belloni
Hi,

On 24/02/2021 15:55:00+0100, Bruno Thomsen wrote:
> You could be right about that, I don't think the watchdog feature should
> be available for use if the alarm feature is enabled due to how CTRL2
> register behaves.
> 
> The hardware I am testing on is a custom board, but it's actually
> possible to get a Raspberry Pi module called RasClock that has
> the chip.
> 

I have an eval board for the PCF2127 (and PCF2129), the OM13513.

> I will test some locking around WD_VAL register access as that is used
> in pcf2127_wdt_ping function.
> 
> My initial test shows that spin_lock_irqsave around regmap calls are not
> a good idea as it result in:
> BUG: scheduling while atomic: watchdog/70/0x0002
> BUG: scheduling while atomic: systemd/1/0x0002
> 

The issue is not only regmap but the fact that i2C and spi accesses are
allowed to sleep.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCHv1 1/6] rtc: m41t80: add support for protected clock

2021-02-22 Thread Alexandre Belloni
On 22/02/2021 22:20:47+0100, Alexandre Belloni wrote:
> On 22/02/2021 18:12:42+0100, Sebastian Reichel wrote:
> > Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The
> > modules SQW clock output defaults to 32768 Hz. This behaviour is
> > used to provide the i.MX6 CKIL clock. Once the RTC driver is probed,
> > the clock is disabled and all i.MX6 functionality depending on
> > the 32 KHz clock has undefined behaviour. On systems using hardware
> > watchdog it seems to likely trigger a lot earlier than configured.
> > 
> > The proper solution would be to describe this dependency in DT,
> > but that will result in a deadlock. The kernel will see, that
> > i.MX6 system clock needs the RTC clock and do probe deferral.
> > But the i.MX6 I2C module never becomes usable without the i.MX6
> > CKIL clock and thus the RTC's clock will not be probed. So from
> > the kernel's perspective this is a chicken-and-egg problem.
> > 
> 
> Reading the previous paragraph, I was going to suggest describing the
> dependency and wondering whether this would cause a circular dependency.
> I guess this will keep being an issue for clocks on an I2C or SPI bus...
> 
> > Technically everything is fine by not touching anything, since
> > the RTC clock correctly enables the clock on reset (i.e. on
> > battery backup power loss) and also the bootloader enables it
> > in case a kernel without this support has been booted.
> > 
> > The 'protected-clocks' property is already in use for some clocks
> > that may not be touched because of firmware limitations and is
> > described in Documentation/devicetree/bindings/clock/clock-bindings.txt.
> > 
> > Signed-off-by: Sebastian Reichel 
> Acked-by: Alexandre Belloni 

Or maybe you expected me to apply the patch, how are the following
patches dependent on this one?


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCHv1 1/6] rtc: m41t80: add support for protected clock

2021-02-22 Thread Alexandre Belloni
On 22/02/2021 18:12:42+0100, Sebastian Reichel wrote:
> Congatec's QMX6 system on module (SoM) uses a m41t62 as RTC. The
> modules SQW clock output defaults to 32768 Hz. This behaviour is
> used to provide the i.MX6 CKIL clock. Once the RTC driver is probed,
> the clock is disabled and all i.MX6 functionality depending on
> the 32 KHz clock has undefined behaviour. On systems using hardware
> watchdog it seems to likely trigger a lot earlier than configured.
> 
> The proper solution would be to describe this dependency in DT,
> but that will result in a deadlock. The kernel will see, that
> i.MX6 system clock needs the RTC clock and do probe deferral.
> But the i.MX6 I2C module never becomes usable without the i.MX6
> CKIL clock and thus the RTC's clock will not be probed. So from
> the kernel's perspective this is a chicken-and-egg problem.
> 

Reading the previous paragraph, I was going to suggest describing the
dependency and wondering whether this would cause a circular dependency.
I guess this will keep being an issue for clocks on an I2C or SPI bus...

> Technically everything is fine by not touching anything, since
> the RTC clock correctly enables the clock on reset (i.e. on
> battery backup power loss) and also the bootloader enables it
> in case a kernel without this support has been booted.
> 
> The 'protected-clocks' property is already in use for some clocks
> that may not be touched because of firmware limitations and is
> described in Documentation/devicetree/bindings/clock/clock-bindings.txt.
> 
> Signed-off-by: Sebastian Reichel 
Acked-by: Alexandre Belloni 

> ---
>  Documentation/devicetree/bindings/rtc/rtc-m41t80.txt | 1 +
>  drivers/rtc/rtc-m41t80.c | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt 
> b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> index c746cb221210..ea4bbf5c4282 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-m41t80.txt
> @@ -19,6 +19,7 @@ Optional properties:
>  - interrupts: rtc alarm interrupt.
>  - clock-output-names: From common clock binding to override the default 
> output
>clock name
> +- protected-clocks: Bool, if set operating system should not handle clock.
>  - wakeup-source: Enables wake up of host system on alarm
>  
>  Example:
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 160dcf68e64e..3296583853a8 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -546,6 +546,9 @@ static struct clk *m41t80_sqw_register_clk(struct 
> m41t80_data *m41t80)
>   struct clk_init_data init;
>   int ret;
>  
> + if (of_property_read_bool(node, "protected-clocks"))
> +     return 0;
> +
>   /* First disable the clock */
>   ret = i2c_smbus_read_byte_data(client, M41T80_REG_ALARM_MON);
>   if (ret < 0)
> -- 
> 2.30.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[GIT PULL] RTC for 5.12

2021-02-21 Thread Alexandre Belloni
Hello Linus,

Here is the RTC subsystem pull request for v5.12. Many cleanups and a
few drivers removal this cycle.

The following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:

  Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git tags/rtc-5.12

for you to fetch changes up to 49dfc1f16b03a6abc17721d4600f7a0bf3d3e4ed:

  rtc: abx80x: Add utility function for writing configuration key (2021-02-13 
23:03:26 +0100)


RTC for 5.12

Subsystem:
 - Introduce features bitfield and the first feature: RTC_FEATURE_ALARM

Removed drivers:
 - ab3100
 - coh901331
 - tx4939
 - sirfsoc

Drivers:
 - use rtc_lock and rtc_unlock instead of opencoding
 - constify all struct rtc_class_ops
 - quiet maybe-unused variable warning
 - replace spin_lock_irqsave with spin_lock in hard IRQ
 - pcf2127: disable Power-On Reset Override and run OTP refresh


Alexandre Belloni (58):
  rtc: opal: set range
  rtc: introduce features bitfield
  rtc: pl031: use RTC_FEATURE_ALARM
  rtc: armada38x: remove armada38x_rtc_ops_noirq
  rtc: cmos: remove cmos_rtc_ops_no_alarm
  rtc: mv: remove mv_rtc_alarm_ops
  rtc: m48t59: remove m48t02_rtc_ops
  rtc: pcf2127: remove pcf2127_rtc_alrm_ops
  rtc: pcf85063: remove pcf85063_rtc_ops_alarm
  rtc: rx8010: drop a struct rtc_class_ops
  rtc: pcf85363: drop a struct rtc_class_ops
  rtc: m41t80: constify m41t80_rtc_ops
  rtc: opal: constify opal_rtc_ops
  rtc: rv3028: constify rv3028_rtc_ops
  rtc: rv3029: constify rv3029_rtc_ops
  rtc: rv3032: constify rv3032_rtc_ops
  rtc: rv8803: constify rv8803_rtc_ops
  rtc: tps65910: remove tps65910_rtc_ops_noirq
  rtc: ac100: use rtc_lock/rtc_unlock
  rtc: asm9260: use rtc_lock/rtc_unlock
  rtc: ds1305: use rtc_lock/rtc_unlock
  rtc: ds1307: use rtc_lock/rtc_unlock
  rtc: ds1685: use rtc_lock/rtc_unlock
  rtc: ds3232: use rtc_lock/rtc_unlock
  rtc: hym8563: use rtc_lock/rtc_unlock
  rtc: m41t80: use rtc_lock/rtc_unlock
  rtc: mcp795: use rtc_lock/rtc_unlock
  rtc: pcf2123: use rtc_lock/rtc_unlock
  rtc: rv3029: use rtc_lock/rtc_unlock
  rtc: rx8010: use rtc_lock/rtc_unlock
  rtc: rx8025: use rtc_lock/rtc_unlock
  rtc: stm32: use rtc_lock/rtc_unlock
  rtc: rv3028: fix PORF handling
  rtc: rv3028: remove useless warning messages
  dt-bindings: rtc: pcf2127: update bindings
  rtc: class: remove bogus documentation
  rtc: armada38x: depend on OF
  rtc: bq32k: quiet maybe-unused variable warning
  rtc: brcmstb-waketimer: quiet maybe-unused variable warning
  rtc: digicolor: quiet maybe-unused variable warning
  rtc: ds1672: quiet maybe-unused variable warning
  rtc: ds3232: quiet maybe-unused variable warning
  rtc: isl1208: quiet maybe-unused variable warning
  rtc: m41t80: quiet maybe-unused variable warning
  rtc: meson: quiet maybe-unused variable warning
  rtc: pcf85063: quiet maybe-unused variable warnings
  rtc: pcf85363: quiet maybe-unused variable warning
  rtc: rs5c372: quiet maybe-unused variable warning
  rtc: rv3028: quiet maybe-unused variable warning
  rtc: rv3029: quiet maybe-unused variable warning
  rtc: rv3032: quiet maybe-unused variable warning
  rtc: rv8803: quiet maybe-unused variable warning
  rtc: rx8010: quiet maybe-unused variable warning
  rtc: rx8581: quiet maybe-unused variable warning
  rtc: s35390a: quiet maybe-unused variable warning
  rtc: sd3078: quiet maybe-unused variable warning
  rtc: s3c: stop setting bogus time
  rtc: s3c: quiet maybe-unused variable warning

Arnd Bergmann (4):
  rtc: rx6110: fix build against modular I2C
  rtc: remove sirfsoc driver
  rtc: remove ste coh901 driver
  rtc: remove ste ab3100 driver

Bartosz Golaszewski (3):
  rtc: s5m: select REGMAP_I2C
  rtc: s5m: use devm_i2c_new_dummy_device()
  rtc: s5m: check the return value of s5m8767_rtc_init_reg()

Biwen Li (1):
  rtc: pcf2127: properly set flag WD_CD for rtc chips(pcf2129, pca2129)

Claudiu Beznea (1):
  dt-bindings: rtc: at91rm9200: add sama7g5 compatible

David Gow (1):
  rtc: zynqmp: depend on HAS_IOMEM

Dmitry Osipenko (1):
  rtc: tps65910: Support wakeup-source property

Guixiong Wei (1):
  rtc: pm8xxx: Read ALARM_EN and update to alarm enabled status

Kevin P. Fleming (1):
  rtc: abx80x: Add utility function for writing configuration key

Marek Vasut (1):
  rtc: pcf8563: Add NXP PCA8565 compatible

Philipp Rosenberger (2):
  rtc: pcf2127: Disable Power-On Reset Override
  rtc: pcf2127: Run a OTP refresh if not done before

Thomas Bogendoerfer (1):
  rtc: tx4939: Remove driver

Xiaofei Tan (6):
  rtc: cmos: Replace

[GIT PULL] I3C changes for 5.12

2021-02-21 Thread Alexandre Belloni
Hello Linus,

Here is the I3C pull request for v5.12 with a few improvements for the
core and a new driver.

The following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:

  Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git tags/i3c/for-5.12

for you to fetch changes up to 5c34b8e7e8bb605925b33e1aa7dc17966811219a:

  i3c: master: dw: Drop redundant disec call (2021-02-06 00:44:15 +0100)


I3C for 5.12

Subsystem:
 - Handle drivers without probe or remove callback
 - Remove callback now returns void
 - DT documentation is now in yaml

New driver:
 - Silvaco I3C master


David Gow (1):
  i3c/master/mipi-i3c-hci: Specify HAS_IOMEM dependency

Miquel Raynal (7):
  dt-bindings: i3c: Convert the bus description to yaml
  dt-bindings: i3c: mipi-hci: Include the bus binding
  dt-bindings: Add vendor prefix for Silvaco
  dt-bindings: i3c: Describe Silvaco master binding
  i3c: master: svc: Add Silvaco I3C master driver
  MAINTAINERS: Add Silvaco I3C master
  i3c: master: dw: Drop redundant disec call

Uwe Kleine-König (2):
  i3c: Handle drivers without probe or remove callback
  i3c: Make remove callback return void

 Documentation/devicetree/bindings/i3c/i3c.txt  |  140 --
 Documentation/devicetree/bindings/i3c/i3c.yaml |  179 +++
 .../devicetree/bindings/i3c/mipi-i3c-hci.yaml  |9 +-
 .../bindings/i3c/silvaco,i3c-master.yaml   |   60 +
 .../devicetree/bindings/vendor-prefixes.yaml   |2 +
 MAINTAINERS|8 +
 drivers/i3c/device.c   |5 +
 drivers/i3c/master.c   |8 +-
 drivers/i3c/master/Kconfig |9 +
 drivers/i3c/master/Makefile|1 +
 drivers/i3c/master/dw-i3c-master.c |5 -
 drivers/i3c/master/svc-i3c-master.c| 1478 
 include/linux/i3c/device.h |2 +-
 13 files changed, 1753 insertions(+), 153 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/i3c/i3c.txt
 create mode 100644 Documentation/devicetree/bindings/i3c/i3c.yaml
 create mode 100644 
Documentation/devicetree/bindings/i3c/silvaco,i3c-master.yaml
 create mode 100644 drivers/i3c/master/svc-i3c-master.c

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [bug] RTC alarm vs system suspend race condition

2021-02-18 Thread Alexandre Belloni
     
> 5fc0:        
> 5fe0:     0013 
> ---[ end trace f97d91a3f84ea22a ]---
> [ cut here ]
> WARNING: CPU: 0 PID: 83 at drivers/mfd/tps6586x.c:266 
> tps6586x_irq_sync_unlock+0x6c/0x70
> Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib xfs libcrc32c fuse 
> crc32_generic f2fs tegra_drm gpu_sched panel_simple tegra20_emc ci_hdrc_tegra 
> host1x_drv iova
> CPU: 0 PID: 83 Comm: kworker/0:2 Tainted: GW 
> 5.11.0-rc2-next-20210106-tegra+ #181
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> Workqueue: events rtc_timer_do_work
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0xc0/0xd4)
> [] (dump_stack) from [] (__warn+0xc0/0x11c)
> [] (__warn) from [] (warn_slowpath_fmt+0x64/0xc0)
> [] (warn_slowpath_fmt) from [] 
> (tps6586x_irq_sync_unlock+0x6c/0x70)
> [] (tps6586x_irq_sync_unlock) from [] 
> (__disable_irq_nosync+0x58/0x88)
> [] (__disable_irq_nosync) from [] (disable_irq+0xc/0x20)
> [] (disable_irq) from [] 
> (tps6586x_rtc_alarm_irq_enable+0x4c/0x58)
> [] (tps6586x_rtc_alarm_irq_enable) from [] 
> (rtc_timer_do_work+0xfc/0x1dc)
> [] (rtc_timer_do_work) from [] 
> (process_one_work+0x1e8/0x44c)
> [] (process_one_work) from [] (worker_thread+0x64/0x5a8)
> [] (worker_thread) from [] (kthread+0x148/0x14c)
> [] (kthread) from [] (ret_from_fork+0x14/0x24)
> Exception stack(0xc59d5fb0 to 0xc59d5ff8)
> 5fa0:    
> 5fc0:        
> 5fe0:     0013 
> ---[ end trace f97d91a3f84ea22b ]---
> [ cut here ]
> WARNING: CPU: 0 PID: 83 at drivers/mfd/tps6586x.c:266 
> tps6586x_irq_sync_unlock+0x6c/0x70
> Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib xfs libcrc32c fuse 
> crc32_generic f2fs tegra_drm gpu_sched panel_simple tegra20_emc ci_hdrc_tegra 
> host1x_drv iova
> CPU: 0 PID: 83 Comm: kworker/0:2 Tainted: GW 
> 5.11.0-rc2-next-20210106-tegra+ #181
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> Workqueue: events rtc_timer_do_work
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0xc0/0xd4)
> [] (dump_stack) from [] (__warn+0xc0/0x11c)
> [] (__warn) from [] (warn_slowpath_fmt+0x64/0xc0)
> [] (warn_slowpath_fmt) from [] 
> (tps6586x_irq_sync_unlock+0x6c/0x70)
> [] (tps6586x_irq_sync_unlock) from [] 
> (__disable_irq_nosync+0x58/0x88)
> [] (__disable_irq_nosync) from [] (disable_irq+0xc/0x20)
> [] (disable_irq) from [] 
> (tps6586x_rtc_alarm_irq_enable+0x4c/0x58)
> [] (tps6586x_rtc_alarm_irq_enable) from [] 
> (rtc_timer_do_work+0xfc/0x1dc)
> [] (rtc_timer_do_work) from [] 
> (process_one_work+0x1e8/0x44c)
> [] (process_one_work) from [] (worker_thread+0x64/0x5a8)
> [] (worker_thread) from [] (kthread+0x148/0x14c)
> [] (kthread) from [] (ret_from_fork+0x14/0x24)
> Exception stack(0xc59d5fb0 to 0xc59d5ff8)
> 5fa0:    
> 5fc0:        
> 5fe0:     0013 
> ---[ end trace f97d91a3f84ea22c ]---
> Enabling non-boot CPUs ...
> [ cut here ]
> WARNING: CPU: 0 PID: 83 at drivers/mfd/tps6586x.c:266 
> tps6586x_irq_sync_unlock+0x6c/0x70
> Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib xfs libcrc32c fuse 
> crc32_generic f2fs tegra_drm gpu_sched panel_simple tegra20_emc ci_hdrc_tegra 
> host1x_drv iova
> CPU: 0 PID: 83 Comm: kworker/0:2 Tainted: GW 
> 5.11.0-rc2-next-20210106-tegra+ #181
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> Workqueue: events rtc_timer_do_work
> [] (unwind_backtrace) from [] (show_stack+0x10/0x14)
> [] (show_stack) from [] (dump_stack+0xc0/0xd4)
> [] (dump_stack) from [] (__warn+0xc0/0x11c)
> [] (__warn) from [] (warn_slowpath_fmt+0x64/0xc0)
> [] (warn_slowpath_fmt) from [] 
> (tps6586x_irq_sync_unlock+0x6c/0x70)
> [] (tps6586x_irq_sync_unlock) from [] 
> (__disable_irq_nosync+0x58/0x88)
> [] (__disable_irq_nosync) from [] (disable_irq+0xc/0x20)
> [] (disable_irq) from [] 
> (tps6586x_rtc_alarm_irq_enable+0x4c/0x58)
> [] (tps6586x_rtc_alarm_irq_enable) from [] 
> (rtc_timer_do_work+0xfc/0x1dc)
> [] (rtc_timer_do_work) from [] 
> (process_one_work+0x1e8/0x44c)
> [] (process_one_work) from [] (worker_thread+0x64/0x5a8)
> [] (worker_thread) from [] (kthread+0x148/0x14c)
> [] (kthread) from [] (ret_from_fork+0x14/0x24)
> Exception stack(0xc59d5fb0 to 0xc59d5ff8)
> 5fa0:    
> 5fc0:        
> 5fe0:     0013 
> ---[ end trace f97d91a3f84ea22d ]---

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v5 1/3] dt-bindings: reset: microchip sparx5 reset driver bindings

2021-02-15 Thread Alexandre Belloni
On 10/02/2021 10:19:50+0100, Steen Hegelund wrote:
> Document the Sparx5 reset device driver bindings
> 
> The driver uses two IO ranges on sparx5 for access to
> the reset control and the reset status.
> 
> Signed-off-by: Steen Hegelund 
> ---
>  .../bindings/reset/microchip,rst.yaml | 55 +++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/microchip,rst.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reset/microchip,rst.yaml 
> b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> new file mode 100644
> index ..80046172c9f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/microchip,rst.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/reset/microchip,rst.yaml#;
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#;
> +
> +title: Microchip Sparx5 Switch Reset Controller
> +
> +maintainers:
> +  - Steen Hegelund 
> +  - Lars Povlsen 
> +
> +description: |
> +  The Microchip Sparx5 Switch provides reset control and implements the 
> following
> +  functions
> +- One Time Switch Core Reset (Soft Reset)
> +
> +properties:
> +  $nodename:
> +pattern: "^reset-controller@[0-9a-f]+$"
> +
> +  compatible:
> +const: microchip,sparx5-switch-reset
> +
> +  reg:
> +items:
> +  - description: cpu block registers
> +  - description: global control block registers
> +
> +  reg-names:
> +items:
> +  - const: cpu
> +  - const: gcb
> +

I still think this is not right because then you will be mapping the
same set of register using multiple drivers without any form of
synchronisation which will work because you are mapping the region
without requesting it. But this may lead to issues later.

At least, you should make cpu start at 0x80 and of size 4. Else, you
won't be able to define and use the GPRs that are in front of
CPU_REGS:RESET.

I would still keep DEVCPU_GCB:CHIP_REGS as a syscon, especially since
you are mapping the whole set of registers.


> +  "#reset-cells":
> +const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#reset-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +reset: reset-controller@0 {
> +compatible = "microchip,sparx5-switch-reset";
> +#reset-cells = <1>;
> +reg = <0x0 0xd0>,
> +  <0x1101 0x1>;
> +reg-names = "cpu", "gcb";
> +};
> +
> -- 
> 2.30.0
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: (subset) [v2 1/2] rtc: pcf2127: properly set flag WD_CD for rtc chips(pcf2129, pca2129)

2021-02-13 Thread Alexandre Belloni
On Wed, 2 Dec 2020 11:18:39 +0800, Biwen Li wrote:
> Properly set flag WD_CD for rtc chips(pcf2129, pca2129)

Applied, thanks!

[1/2] rtc: pcf2127: properly set flag WD_CD for rtc chips(pcf2129, pca2129)
  commit: 2843d565dd78fd9117b9a18567cf68ac37a5dd1e

Finally, I did revert back to your first version, after renaming has_nvmem.

Best regards,
-- 
Alexandre Belloni 


Re: [PATCH] pinctrl: PINCTRL_MICROCHIP_SGPIO should depend on ARCH_SPARX5 || SOC_VCOREIII

2021-02-10 Thread Alexandre Belloni
On 10/02/2021 14:53:01+0100, Geert Uytterhoeven wrote:
> Hi Lars,
> 
> On Wed, Feb 10, 2021 at 2:45 PM Lars Povlsen  
> wrote:
> > Geert Uytterhoeven writes:
> > > the Microsemi/Microchip Serial GPIO device is present only Microsemi
> > > VCore III and Microchip Sparx5 SoCs.  Hence add a dependency on
> > > ARCH_SPARX5 || SOC_VCOREIII, to prevent asking the user about this
> > > driver when configuring a kernel without support for these SoCs.
> > >
> > > Fixes: 7e5ea974e61c8dd0 ("pinctrl: pinctrl-microchip-sgpio: Add pinctrl 
> > > driver for Microsemi Serial GPIO")
> > > Signed-off-by: Geert Uytterhoeven 
> > > ---
> > >  drivers/pinctrl/Kconfig | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > > index 113073d5f89bbf70..3b75b1d7d3d1f1b0 100644
> > > --- a/drivers/pinctrl/Kconfig
> > > +++ b/drivers/pinctrl/Kconfig
> > > @@ -353,8 +353,8 @@ config PINCTRL_OCELOT
> > >
> > >  config PINCTRL_MICROCHIP_SGPIO
> > > bool "Pinctrl driver for Microsemi/Microchip Serial GPIO"
> > > -   depends on OF
> > > -   depends on HAS_IOMEM
> > > +   depends on OF && HAS_IOMEM
> > > +   depends on ARCH_SPARX5 || SOC_VCOREIII || COMPILE_TEST
> > > select GPIOLIB
> > > select GPIOLIB_IRQCHIP
> > > select GENERIC_PINCONF
> >
> > Thank you for your patch. Unfortunately, it makes it impossible to use
> > the driver across PCIe - which is a specifically desired configuration.
> >
> > Could you add CONFIG_PCI to the || chain?
> 
> Sure.
> 
> Is PCIe the only other transport over which the register can be accessed?
> Or can this also be done over e.g. SPI, like on Ocelot[1]?
> 
> [1] https://lore.kernel.org/linux-gpio/20200511145329.gv34...@piout.net/
> 

Yes, this driver IP is also available on Ocelot (this is SOC_VCOREIII)
so this is also available over SPI.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[GIT PULL] I3C fixes for 5.11

2021-02-09 Thread Alexandre Belloni
Hello Linus,

Here is a single compilation warning fix for v5.11.

The following changes since commit 5c8fe583cce542aa0b84adc939ce85293de36e5e:

  Linux 5.11-rc1 (2020-12-27 15:30:22 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git 
tags/i3c/fixes-for-5.11

for you to fetch changes up to 291b5c9870fc546376d69cf792b7885cd0c9c1b3:

  i3c/master/mipi-i3c-hci: Fix position of __maybe_unused in i3c_hci_of_match 
(2020-12-31 18:41:37 +0100)


I3C fixes for 5.11

Drivers:
 - mipi-i3c-hci: fix compilation warning with Clang


Nathan Chancellor (1):
  i3c/master/mipi-i3c-hci: Fix position of __maybe_unused in 
i3c_hci_of_match

 drivers/i3c/master/mipi-i3c-hci/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [GIT PULL 2/3] ARM: dts: samsung: DTS for v5.12

2021-02-08 Thread Alexandre Belloni
On 08/02/2021 23:14:02+0100, Arnd Bergmann wrote:
> On Mon, Feb 8, 2021 at 10:35 PM Alexandre Belloni
>  wrote:
> > On 08/02/2021 20:52:37+0100, Arnd Bergmann wrote:
> > > On Mon, Feb 8, 2021 at 7:42 PM Krzysztof Kozlowski  
> > > wrote:
> > > > Let me steer the discussion to original topic - it's about old kernel
> > > > and new DTB, assuming that mainline kernel bisectability is not
> > > > affected.
> > > >
> > > > Flow looks like this:
> > > >
> > > > 0. You have existing bidings and drivers.
> > > > 1. Patch changing bindings (with new compatible) and drivers gets
> > > >accepted by maintainer.
> > > > 2. Patch above (bindings+drivers) goes during merge window to v5.11-rc1.
> > > > 3. Patch changing in-tree DTS to new compatible gets accepted by
> > > >maintainer and it is sent as v5.12-rc1 material to SoC maintainers.
> > > >
> > > > So again: old kernel, using old bindings, new DTB.
> > > >
> >
> > I don't think forward compatibility was ever considered. I've seen it
> > being mentioned a few times on #armlinux but honestly this simply can't
> > be achieved. This would mean being able to write complete DT bindings
> > for a particular SoC at day 0 which will realistically never happen. You
> > may noteven have a complete datasheet and even if you have a datasheet,
> > it may not be complete or it may be missing hw errata that are
> > discovered later on and need a new binding to handle.
> 
> You do not have to write the correct DT for this, the only requirement
> is that any changes to a node are backward-compatible, which is
> typically the case if you add properties or compatible strings without
> removing the old one. A bugfix in this case is also backward-compatible.
> 
> The part that can not happen instead is to write a DT that can expose
> features that any future kernel will use.
> 

But I think we are speaking about the other way around were you would be
e.g. removing properties or splitting a node is multiple different
nodes following a different understanding of the hardware.
And in this case, any rework of the bindings will be forbidden, like
32b7cfbd4bb2 ("ARM: dts: at91: remove deprecated ADC properties") will
break older kernels trying to use the new dtb.
761f6ed85417 ("ARM: dts: at91: sama5d4: use correct rtc compatible") is
an other case.
I'm not sure want to keep the older properties or the older compatible
string as a fallback for this use case.

> > > However, once the firmware is updated, it may no longer be possible to
> > > go back to the old kernel in case the new one is busted.
> > >
> >
> > Any serious update strategy will update both the kernel and device tree
> > at the same time, exactly like you already have to update the initramfs
> > with the kernel as soon as it is including kernel modules.
> > I would expect any embedded platform to actually use a container format,
> > like a FIT image that will ship the kernel, DT and intiramfs in a single
> > image and will allow to sign all parts.
> 
> Embedded systems that do this have no requirement for backward
> or forward compatibility at all, the only requirement for these is 
> bisectability
> of git commits.
> 

Yes and I can't see any drawbacks in this approach.

> > > A similar problem can happen with the EBBR boot flow that relies on
> > > a uefi-enabled firmware such as a u-boot, while using grub2 as the
> > > actual boot loader. This is commonly supported across distros. While
> > > grub2 can load a matching set of kernel+initrd+dtb from disk and run
> > > that, this often fails in practice because u-boot needs to fill a
> > > board specific set of DT properties (bootargs, detected memory,
> > > mac address, ...). The usual way this gets handled is that u-boot loads
> > > grub2 and the dtb from disk and then passes the modified dtb to grub,
> > > which picks only kernel+initrd from disk and boots this with the dtb.
> > >
> > > The result is similar to case with dtb built into the firmware: after
> > > upgrading the dtb that gets loaded by u-boot, grub can still pick
> > > old kernels but they may not work as they did in the past. There are
> > > obviously ways to work around it, but it does lead to user frustration.
> > >
> >
> > Are there really any platforms with the dtb built into the firmware?
> > I feel like this is a mythical creature used to scare people into keeping
> > the DTB ABI stable. Aren't all the distribution already able to cope
> >

Re: WARNING in mc1NUM_get_time

2021-02-08 Thread Alexandre Belloni
07f665e7a5218 EFLAGS: 0246 ORIG_RAX: 00ca
> RAX: fe00 RBX: 0056bf68 RCX: 00465b09
> RDX:  RSI: 0080 RDI: 0056bf68
> RBP: 0056bf60 R08:  R09: 
> R10:  R11: 0246 R12: 0056bf6c
> R13: 7ffeb1baab0f R14: 7f665e7a5300 R15: 00022000
> 
> 
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkal...@googlegroups.com.
> 
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [GIT PULL 2/3] ARM: dts: samsung: DTS for v5.12

2021-02-08 Thread Alexandre Belloni
Hello,

On 08/02/2021 20:52:37+0100, Arnd Bergmann wrote:
> On Mon, Feb 8, 2021 at 7:42 PM Krzysztof Kozlowski  wrote:
> > Let me steer the discussion to original topic - it's about old kernel
> > and new DTB, assuming that mainline kernel bisectability is not
> > affected.
> >
> > Flow looks like this:
> >
> > 0. You have existing bidings and drivers.
> > 1. Patch changing bindings (with new compatible) and drivers gets
> >accepted by maintainer.
> > 2. Patch above (bindings+drivers) goes during merge window to v5.11-rc1.
> > 3. Patch changing in-tree DTS to new compatible gets accepted by
> >maintainer and it is sent as v5.12-rc1 material to SoC maintainers.
> >
> > So again: old kernel, using old bindings, new DTB.
> >

I don't think forward compatibility was ever considered. I've seen it
being mentioned a few times on #armlinux but honestly this simply can't
be achieved. This would mean being able to write complete DT bindings
for a particular SoC at day 0 which will realistically never happen. You
may noteven have a complete datasheet and even if you have a datasheet,
it may not be complete or it may be missing hw errata that are
discovered later on and need a new binding to handle.

> > Another case is where out-of-tree user of bindings, e.g. FreeBSD, takes
> > new DTS (at point of #3 above or later) but did not take the bindings.
> > Such system would be broken but it's their fault - they took DTS without
> > taking the bindings (which were there already for one release!).
> 
> The particular boot flow that I am worried about here is when the dtb and
> kernel are not updated in sync. Traditionally this happens when the dtb
> is contained in the firmware image or generated by the firmware, as would
> be the case on any server class system, but also on embedded systems
> that can run an upstream kernel but without having the dts contributed
> into the kernel sources.
> 
> When you have this case, you can install a working system, and install
> an upgraded kernel without problems. You might then want to update the
> firmware as well, in order to take advantage of the features implemented
> in the kernel kernel that require a DT description. Again, no problem.
> 
> However, once the firmware is updated, it may no longer be possible to
> go back to the old kernel in case the new one is busted.
> 

Any serious update strategy will update both the kernel and device tree
at the same time, exactly like you already have to update the initramfs
with the kernel as soon as it is including kernel modules.
I would expect any embedded platform to actually use a container format,
like a FIT image that will ship the kernel, DT and intiramfs in a single
image and will allow to sign all parts.

> A similar problem can happen with the EBBR boot flow that relies on
> a uefi-enabled firmware such as a u-boot, while using grub2 as the
> actual boot loader. This is commonly supported across distros. While
> grub2 can load a matching set of kernel+initrd+dtb from disk and run
> that, this often fails in practice because u-boot needs to fill a
> board specific set of DT properties (bootargs, detected memory,
> mac address, ...). The usual way this gets handled is that u-boot loads
> grub2 and the dtb from disk and then passes the modified dtb to grub,
> which picks only kernel+initrd from disk and boots this with the dtb.
> 
> The result is similar to case with dtb built into the firmware: after
> upgrading the dtb that gets loaded by u-boot, grub can still pick
> old kernels but they may not work as they did in the past. There are
> obviously ways to work around it, but it does lead to user frustration.
> 

Are there really any platforms with the dtb built into the firmware? I
feel like this is a mythical creature used to scare people into keeping
the DTB ABI stable. Aren't all the distribution already able to cope
with keeping DTB and kernel in sync?

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


Re: [PATCH v2 1/2] iio: hid-sensors: Move get sensitivity attribute to hid-sensor-common

2021-02-06 Thread Alexandre Belloni
On 01/02/2021 13:49:20+0800, Ye Xiang wrote:
> No functional change has been made with this patch. The main intent here
> is to reduce code repetition of getting sensitivity attribute.
> 
> In the current implementation, sensor_hub_input_get_attribute_info() is
> called from multiple drivers to get attribute info for sensitivity
> field. Moving this to common place will avoid code repetition.
> 
> Signed-off-by: Ye Xiang 
Acked-by: Alexandre Belloni 

> ---
>  drivers/iio/accel/hid-sensor-accel-3d.c   | 23 ++---
>  .../hid-sensors/hid-sensor-attributes.c   | 17 +-
>  drivers/iio/gyro/hid-sensor-gyro-3d.c | 19 ---
>  drivers/iio/humidity/hid-sensor-humidity.c| 16 --
>  drivers/iio/light/hid-sensor-als.c| 19 ---
>  drivers/iio/light/hid-sensor-prox.c   | 27 +---
>  drivers/iio/magnetometer/hid-sensor-magn-3d.c | 32 ++-
>  drivers/iio/orientation/hid-sensor-incl-3d.c  | 19 ---
>  drivers/iio/orientation/hid-sensor-rotation.c | 23 ++---
>  .../position/hid-sensor-custom-intel-hinge.c  | 20 
>  drivers/iio/pressure/hid-sensor-press.c   | 19 ---
>  .../iio/temperature/hid-sensor-temperature.c  | 16 --
>  drivers/rtc/rtc-hid-sensor-time.c |  4 ++-
>  include/linux/hid-sensor-hub.h|  4 ++-
>  14 files changed, 107 insertions(+), 151 deletions(-)
> 
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c 
> b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 5d63ed19e6e2..2f9465cb382f 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
> @@ -43,6 +43,10 @@ static const u32 accel_3d_addresses[ACCEL_3D_CHANNEL_MAX] 
> = {
>   HID_USAGE_SENSOR_ACCEL_Z_AXIS
>  };
>  
> +static const u32 accel_3d_sensitivity_addresses[] = {
> + HID_USAGE_SENSOR_DATA_ACCELERATION,
> +};
> +
>  /* Channel definitions */
>  static const struct iio_chan_spec accel_3d_channels[] = {
>   {
> @@ -317,18 +321,6 @@ static int accel_3d_parse_report(struct platform_device 
> *pdev,
>   >accel[CHANNEL_SCAN_INDEX_X],
>   >scale_pre_decml, >scale_post_decml);
>  
> - /* Set Sensitivity field ids, when there is no individual modifier */
> - if (st->common_attributes.sensitivity.index < 0) {
> - sensor_hub_input_get_attribute_info(hsdev,
> - HID_FEATURE_REPORT, usage_id,
> - HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
> - HID_USAGE_SENSOR_DATA_ACCELERATION,
> - >common_attributes.sensitivity);
> - dev_dbg(>dev, "Sensitivity index:report %d:%d\n",
> - st->common_attributes.sensitivity.index,
> - st->common_attributes.sensitivity.report_id);
> - }
> -
>   return ret;
>  }
>  
> @@ -366,8 +358,11 @@ static int hid_accel_3d_probe(struct platform_device 
> *pdev)
>   channel_size = sizeof(gravity_channels);
>   indio_dev->num_channels = ARRAY_SIZE(gravity_channels);
>   }
> - ret = hid_sensor_parse_common_attributes(hsdev, hsdev->usage,
> - _state->common_attributes);
> + ret = hid_sensor_parse_common_attributes(hsdev,
> +  hsdev->usage,
> +  
> _state->common_attributes,
> +  accel_3d_sensitivity_addresses,
> +  
> ARRAY_SIZE(accel_3d_sensitivity_addresses));
>   if (ret) {
>   dev_err(>dev, "failed to setup common attributes\n");
>   return ret;
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c 
> b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index 5b822a4298a0..d349ace2e33f 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> @@ -448,12 +448,15 @@ EXPORT_SYMBOL(hid_sensor_batch_mode_supported);
>  
>  int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
>   u32 usage_id,
> - struct hid_sensor_common *st)
> + struct hid_sensor_common *st,
> + const u32 *sensitivity_addresses,
> + u32 sensitivity_addresses_len)
>  {
>  
>   struct hid_sensor_hub_a

Re: [PATCH 0/6] spin lock usage optimization for RTC drivers

2021-02-05 Thread Alexandre Belloni
On Wed, 3 Feb 2021 20:39:35 +0800, Xiaofei Tan wrote:
> Replace spin_lock_irqsave with spin_lock in hard IRQ of RTC drivers.
> There is no function changes, but may speed up if interrupt happen
> too often.
> 
> Xiaofei Tan (6):
>   rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard IRQ
>   rtc: pm8xxx: Replace spin_lock_irqsave with spin_lock in hard IRQ
>   rtc: r7301: Replace spin_lock_irqsave with spin_lock in hard IRQ
>   rtc: tegra: Replace spin_lock_irqsave with spin_lock in hard IRQ
>   rtc: mxc: Replace spin_lock_irqsave with spin_lock in hard IRQ
>   rtc: mxc_v2: Replace spin_lock_irqsave with spin_lock in hard IRQ
> 
> [...]

Applied, thanks!

[1/6] rtc: cmos: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: 6950d046eb6eabbc271fda416460c05f7a85698a
[2/6] rtc: pm8xxx: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: 51317975565329ee50e52d0fffce50566b43cc2d
[3/6] rtc: r7301: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: be3df3f85897e36163bfb764549acc69ec9b7afa
[4/6] rtc: tegra: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: 669022c29af672205aaf53b76fd594dad2661ffe
[5/6] rtc: mxc: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: 3f2d30184773e408c4e6f9e15c350828e482480c
[6/6] rtc: mxc_v2: Replace spin_lock_irqsave with spin_lock in hard IRQ
  commit: 0c1095d334dafda22463454b0519c926447b555e

Best regards,
-- 
Alexandre Belloni 


[tip: timers/core] alarmtimer: Update kerneldoc

2021-02-05 Thread tip-bot2 for Alexandre Belloni
The following commit has been merged into the timers/core branch of tip:

Commit-ID: b5c28ea601b801d0ecd5ec703b8d54f77bfe5365
Gitweb:
https://git.kernel.org/tip/b5c28ea601b801d0ecd5ec703b8d54f77bfe5365
Author:Alexandre Belloni 
AuthorDate:Tue, 02 Feb 2021 02:34:57 +01:00
Committer: Thomas Gleixner 
CommitterDate: Fri, 05 Feb 2021 19:26:41 +01:00

alarmtimer: Update kerneldoc

Update kerneldoc comments to reflect the actual arguments and return values
of the documented functions.

Signed-off-by: Alexandre Belloni 
Signed-off-by: Thomas Gleixner 
Link: 
https://lore.kernel.org/r/20210202013457.3482388-1-alexandre.bell...@bootlin.com

---
 kernel/time/alarmtimer.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index f4ace1b..98d7a15 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -527,8 +527,11 @@ static enum alarmtimer_type clock2alarm(clockid_t clockid)
 /**
  * alarm_handle_timer - Callback for posix timers
  * @alarm: alarm that fired
+ * @now: time at the timer expiration
  *
  * Posix timer callback for expired alarm timers.
+ *
+ * Return: whether the timer is to be restarted
  */
 static enum alarmtimer_restart alarm_handle_timer(struct alarm *alarm,
ktime_t now)
@@ -715,8 +718,11 @@ static int alarm_timer_create(struct k_itimer *new_timer)
 /**
  * alarmtimer_nsleep_wakeup - Wakeup function for alarm_timer_nsleep
  * @alarm: ptr to alarm that fired
+ * @now: time at the timer expiration
  *
  * Wakes up the task that set the alarmtimer
+ *
+ * Return: ALARMTIMER_NORESTART
  */
 static enum alarmtimer_restart alarmtimer_nsleep_wakeup(struct alarm *alarm,
ktime_t now)
@@ -733,6 +739,7 @@ static enum alarmtimer_restart 
alarmtimer_nsleep_wakeup(struct alarm *alarm,
  * alarmtimer_do_nsleep - Internal alarmtimer nsleep implementation
  * @alarm: ptr to alarmtimer
  * @absexp: absolute expiration time
+ * @type: alarm type (BOOTTIME/REALTIME).
  *
  * Sets the alarm timer and sleeps until it is fired or interrupted.
  */
@@ -806,7 +813,6 @@ static long __sched alarm_timer_nsleep_restart(struct 
restart_block *restart)
  * @which_clock: clockid
  * @flags: determins abstime or relative
  * @tsreq: requested sleep time (abs or rel)
- * @rmtp: remaining sleep time saved
  *
  * Handles clock_nanosleep calls against _ALARM clockids
  */


Re: [PATCH] drivers: soc: atmel: fix type for same7

2021-02-04 Thread Alexandre Belloni
On 04/02/2021 16:49:25+0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> A missing comma caused a build failure:
> 
> drivers/soc/atmel/soc.c:196:24: error: too few arguments provided to 
> function-like macro invocation
> 
> Fixes: af3a10513cd6 ("drivers: soc: atmel: add per soc id and version match 
> masks")
> Signed-off-by: Arnd Bergmann 
Acked-by: Alexandre Belloni 

> ---
> It is broken in the soc tree at the moment, I can pick up
> the fix directly if I get an Ack
> ---
>  drivers/soc/atmel/soc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/atmel/soc.c b/drivers/soc/atmel/soc.c
> index a2967846809f..a490ad7e090f 100644
> --- a/drivers/soc/atmel/soc.c
> +++ b/drivers/soc/atmel/soc.c
> @@ -191,7 +191,7 @@ static const struct at91_soc socs[] __initconst = {
>   AT91_SOC(SAME70Q20_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
>AT91_CIDR_VERSION_MASK, SAME70Q20_EXID_MATCH,
>"same70q20", "same7"),
> - AT91_SOC(SAME70Q19_CIDR_MATCH, AT91_CIDR_MATCH_MASK
> + AT91_SOC(SAME70Q19_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
>AT91_CIDR_VERSION_MASK, SAME70Q19_EXID_MATCH,
>    "same70q19", "same7"),
>   AT91_SOC(SAMS70Q21_CIDR_MATCH, AT91_CIDR_MATCH_MASK,
> -- 
> 2.29.2
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


[PATCH RESEND v2] ARM: dts: lpc32xx: Revert set default clock rate of HCLK PLL

2021-02-03 Thread Alexandre Belloni
This reverts commit c17e9377aa81664d94b4f2102559fcf2a01ec8e7.

The lpc32xx clock driver is not able to actually change the PLL rate as
this would require reparenting ARM_CLK, DDRAM_CLK, PERIPH_CLK to SYSCLK,
then stop the PLL, update the register, restart the PLL and wait for the
PLL to lock and finally reparent ARM_CLK, DDRAM_CLK, PERIPH_CLK to HCLK
PLL.

Currently, the HCLK driver simply updates the registers but this has no
real effect and all the clock rate calculation end up being wrong. This is
especially annoying for the peripheral (e.g. UARTs, I2C, SPI).

Signed-off-by: Alexandre Belloni 
Tested-by: Gregory CLEMENT 
---
Arnd,

This is a very important fix that was sent back in may and october 2019 without
any reply from the maintainers, please consider applying it so it can be
backported on v5.10.

 arch/arm/boot/dts/lpc32xx.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/boot/dts/lpc32xx.dtsi b/arch/arm/boot/dts/lpc32xx.dtsi
index 3a5cfb0ddb20..c87066d6c995 100644
--- a/arch/arm/boot/dts/lpc32xx.dtsi
+++ b/arch/arm/boot/dts/lpc32xx.dtsi
@@ -326,9 +326,6 @@ clk: clock-controller@0 {
 
clocks = <_32k>, <>;
clock-names = "xtal_32k", "xtal";
-
-   assigned-clocks = < 
LPC32XX_CLK_HCLK_PLL>;
-   assigned-clock-rates = <20800>;
};
};
 
-- 
2.29.2



Re: [PATCH] drivers: rtc: Make xilinx zynqmp driver depend on HAS_IOMEM

2021-02-02 Thread Alexandre Belloni
On Tue, 26 Jan 2021 19:51:47 -0800, David Gow wrote:
> The Xilinx zynqmp RTC driver makes use of IOMEM functions like
> devm_platform_ioremap_resource(), which are only available if
> CONFIG_HAS_IOMEM is defined.
> 
> This causes the driver not to be enable under make ARCH=um allyesconfig,
> even though it won't build.
> 
> [...]

Applied, thanks!

[1/1] drivers: rtc: Make xilinx zynqmp driver depend on HAS_IOMEM
  commit: ddd0521549a975e6148732d6ca6b89ffa862c0e5

Best regards,
-- 
Alexandre Belloni 


[PATCH 1/2] rtc: s3c: stop setting bogus time

2021-02-02 Thread Alexandre Belloni
It doesn't make sense to set the RTC to a default value at probe time. Let
the core handle invalid date and time.

Also, this is basically dead code since commit 22652ba72453 ("rtc: stop
validating rtc_time in .read_time")

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-s3c.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index fab326ba9cec..f07b0c43aafe 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -382,7 +382,6 @@ static int s3c_rtc_remove(struct platform_device *pdev)
 static int s3c_rtc_probe(struct platform_device *pdev)
 {
struct s3c_rtc *info = NULL;
-   struct rtc_time rtc_tm;
int ret;
 
info = devm_kzalloc(>dev, sizeof(*info), GFP_KERNEL);
@@ -448,20 +447,6 @@ static int s3c_rtc_probe(struct platform_device *pdev)
 
device_init_wakeup(>dev, 1);
 
-   /* Check RTC Time */
-   if (s3c_rtc_gettime(>dev, _tm)) {
-   rtc_tm.tm_year  = 100;
-   rtc_tm.tm_mon   = 0;
-   rtc_tm.tm_mday  = 1;
-   rtc_tm.tm_hour  = 0;
-   rtc_tm.tm_min   = 0;
-   rtc_tm.tm_sec   = 0;
-
-   s3c_rtc_settime(>dev, _tm);
-
-   dev_warn(>dev, "warning: invalid RTC value so 
initializing it\n");
-   }
-
/* register RTC and exit */
info->rtc = devm_rtc_device_register(>dev, "s3c", _rtcops,
 THIS_MODULE);
-- 
2.29.2



[PATCH 2/2] rtc: s3c: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-s3c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index f07b0c43aafe..e57d3ca70a78 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -558,7 +558,7 @@ static struct s3c_rtc_data const s3c6410_rtc_data = {
.disable= s3c6410_rtc_disable,
 };
 
-static const struct of_device_id s3c_rtc_dt_match[] = {
+static const __maybe_unused struct of_device_id s3c_rtc_dt_match[] = {
{
.compatible = "samsung,s3c2410-rtc",
.data = _rtc_data,
-- 
2.29.2



[PATCH 01/21] rtc: class: remove bogus documentation

2021-02-02 Thread Alexandre Belloni
rtc_device_unregister is gone since commit fdcfd854333b ("rtc: rework
rtc_register_device() resource management"). Remove its documentation.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/class.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 03abd0aa229a..f77bc089eb6b 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -324,11 +324,6 @@ static void rtc_device_get_offset(struct rtc_device *rtc)
rtc->offset_secs = 0;
 }
 
-/**
- * rtc_device_unregister - removes the previously registered RTC class device
- *
- * @rtc: the RTC class device to destroy
- */
 static void devm_rtc_unregister_device(void *data)
 {
struct rtc_device *rtc = data;
-- 
2.29.2



[PATCH 00/21] rtc: remove make W=1 warnings

2021-02-02 Thread Alexandre Belloni
Hi,

This series removes some make W=1 warning, especially when CONFIG_OF is
not defined.

Alexandre Belloni (21):
  rtc: class: remove bogus documentation
  rtc: armada38x: depend on OF
  rtc: bq32k: quiet maybe-unused variable warning
  rtc: brcmstb-waketimer: quiet maybe-unused variable warning
  rtc: digicolor: quiet maybe-unused variable warning
  rtc: ds1672: quiet maybe-unused variable warning
  rtc: ds3232: quiet maybe-unused variable warning
  rtc: isl1208: quiet maybe-unused variable warning
  rtc: m41t80: quiet maybe-unused variable warning
  rtc: meson: quiet maybe-unused variable warning
  rtc: pcf85063: quiet maybe-unused variable warnings
  rtc: pcf85363: quiet maybe-unused variable warning
  rtc: rs5c372: quiet maybe-unused variable warning
  rtc: rv3028: quiet maybe-unused variable warning
  rtc: rv3029: quiet maybe-unused variable warning
  rtc: rv3032: quiet maybe-unused variable warning
  rtc: rv8803: quiet maybe-unused variable warning
  rtc: rx8010: quiet maybe-unused variable warning
  rtc: rx8581: quiet maybe-unused variable warning
  rtc: s35390a: quiet maybe-unused variable warning
  rtc: sd3078: quiet maybe-unused variable warning

 drivers/rtc/Kconfig |  1 +
 drivers/rtc/class.c |  5 
 drivers/rtc/rtc-bq32k.c |  2 +-
 drivers/rtc/rtc-brcmstb-waketimer.c |  2 +-
 drivers/rtc/rtc-digicolor.c |  2 +-
 drivers/rtc/rtc-ds1672.c|  2 +-
 drivers/rtc/rtc-ds3232.c|  2 +-
 drivers/rtc/rtc-isl1208.c   |  2 +-
 drivers/rtc/rtc-m41t80.c|  2 +-
 drivers/rtc/rtc-meson.c |  2 +-
 drivers/rtc/rtc-pcf85063.c  | 38 ++---
 drivers/rtc/rtc-pcf85363.c  |  2 +-
 drivers/rtc/rtc-rs5c372.c   |  2 +-
 drivers/rtc/rtc-rv3028.c|  2 +-
 drivers/rtc/rtc-rv3029c2.c  |  2 +-
 drivers/rtc/rtc-rv3032.c|  2 +-
 drivers/rtc/rtc-rv8803.c|  2 +-
 drivers/rtc/rtc-rx8010.c|  2 +-
 drivers/rtc/rtc-rx8581.c|  2 +-
 drivers/rtc/rtc-s35390a.c   |  2 +-
 drivers/rtc/rtc-sd3078.c|  2 +-
 21 files changed, 38 insertions(+), 42 deletions(-)

-- 
2.29.2



[PATCH 03/21] rtc: bq32k: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-bq32k.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
index 933e4237237d..2235c968842d 100644
--- a/drivers/rtc/rtc-bq32k.c
+++ b/drivers/rtc/rtc-bq32k.c
@@ -311,7 +311,7 @@ static const struct i2c_device_id bq32k_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, bq32k_id);
 
-static const struct of_device_id bq32k_of_match[] = {
+static const __maybe_unused struct of_device_id bq32k_of_match[] = {
{ .compatible = "ti,bq32000" },
{ }
 };
-- 
2.29.2



[PATCH 1/2] rtc: rv3028: fix PORF handling

2021-02-02 Thread Alexandre Belloni
The PORF bit is cleared on interrupts which prevents the driver to know
when the time and date are invalid. Stop clearing PORF in the interrupt
handler.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv3028.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c
index f68baa055a0b..6d07f8261c69 100644
--- a/drivers/rtc/rtc-rv3028.c
+++ b/drivers/rtc/rtc-rv3028.c
@@ -268,6 +268,8 @@ static irqreturn_t rv3028_handle_irq(int irq, void *dev_id)
if (status & RV3028_STATUS_PORF)
dev_warn(>rtc->dev, "Voltage low, data loss 
detected.\n");
 
+   status &= ~RV3028_STATUS_PORF;
+
if (status & RV3028_STATUS_TF) {
status |= RV3028_STATUS_TF;
ctrl |= RV3028_CTRL2_TIE;
-- 
2.29.2



[PATCH 07/21] rtc: ds3232: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds3232.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds3232.c b/drivers/rtc/rtc-ds3232.c
index 535e4a88fbb6..168bc27f1f5a 100644
--- a/drivers/rtc/rtc-ds3232.c
+++ b/drivers/rtc/rtc-ds3232.c
@@ -592,7 +592,7 @@ static const struct i2c_device_id ds3232_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ds3232_id);
 
-static const struct of_device_id ds3232_of_match[] = {
+static const  __maybe_unused struct of_device_id ds3232_of_match[] = {
{ .compatible = "dallas,ds3232" },
{ }
 };
-- 
2.29.2



[PATCH 2/2] rtc: rv3028: remove useless warning messages

2021-02-02 Thread Alexandre Belloni
Remove voltage low messages as userspace has a proper way to get the
information and react on it.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv3028.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c
index 6d07f8261c69..0c48d980d06a 100644
--- a/drivers/rtc/rtc-rv3028.c
+++ b/drivers/rtc/rtc-rv3028.c
@@ -265,9 +265,6 @@ static irqreturn_t rv3028_handle_irq(int irq, void *dev_id)
return IRQ_NONE;
}
 
-   if (status & RV3028_STATUS_PORF)
-   dev_warn(>rtc->dev, "Voltage low, data loss 
detected.\n");
-
status &= ~RV3028_STATUS_PORF;
 
if (status & RV3028_STATUS_TF) {
@@ -313,10 +310,8 @@ static int rv3028_get_time(struct device *dev, struct 
rtc_time *tm)
if (ret < 0)
return ret;
 
-   if (status & RV3028_STATUS_PORF) {
-   dev_warn(dev, "Voltage low, data is invalid.\n");
+   if (status & RV3028_STATUS_PORF)
return -EINVAL;
-   }
 
ret = regmap_bulk_read(rv3028->regmap, RV3028_SEC, date, sizeof(date));
if (ret)
@@ -828,9 +823,6 @@ static int rv3028_probe(struct i2c_client *client)
if (ret < 0)
return ret;
 
-   if (status & RV3028_STATUS_PORF)
-   dev_warn(>dev, "Voltage low, data loss detected.\n");
-
if (status & RV3028_STATUS_AF)
dev_warn(>dev, "An alarm may have been missed.\n");
 
-- 
2.29.2



[PATCH 06/21] rtc: ds1672: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-ds1672.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
index 630493759d15..4cd8efbef6cf 100644
--- a/drivers/rtc/rtc-ds1672.c
+++ b/drivers/rtc/rtc-ds1672.c
@@ -139,7 +139,7 @@ static const struct i2c_device_id ds1672_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ds1672_id);
 
-static const struct of_device_id ds1672_of_match[] = {
+static const __maybe_unused struct of_device_id ds1672_of_match[] = {
{ .compatible = "dallas,ds1672" },
{ }
 };
-- 
2.29.2



[PATCH 08/21] rtc: isl1208: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-isl1208.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index 563a6d9c9fcf..182dfa605515 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -99,7 +99,7 @@ static const struct i2c_device_id isl1208_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, isl1208_id);
 
-static const struct of_device_id isl1208_of_match[] = {
+static const __maybe_unused struct of_device_id isl1208_of_match[] = {
{ .compatible = "isil,isl1208", .data = _configs[TYPE_ISL1208] 
},
{ .compatible = "isil,isl1209", .data = _configs[TYPE_ISL1209] 
},
{ .compatible = "isil,isl1218", .data = _configs[TYPE_ISL1218] 
},
-- 
2.29.2



[PATCH 10/21] rtc: meson: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-meson.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-meson.c b/drivers/rtc/rtc-meson.c
index 8642c06565ea..44bdc8b4a90d 100644
--- a/drivers/rtc/rtc-meson.c
+++ b/drivers/rtc/rtc-meson.c
@@ -380,7 +380,7 @@ static int meson_rtc_probe(struct platform_device *pdev)
return ret;
 }
 
-static const struct of_device_id meson_rtc_dt_match[] = {
+static const __maybe_unused struct of_device_id meson_rtc_dt_match[] = {
{ .compatible = "amlogic,meson6-rtc", },
{ .compatible = "amlogic,meson8-rtc", },
{ .compatible = "amlogic,meson8b-rtc", },
-- 
2.29.2



[PATCH 11/21] rtc: pcf85063: quiet maybe-unused variable warnings

2021-02-02 Thread Alexandre Belloni
pcf85063a_config and rv8263_config are only referenced by
pcf85063_of_match, move them in the #ifdef CONFIG_OF section.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-pcf85063.c | 38 +++---
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
index f7e7c9eb0781..aef6c1ee8bb0 100644
--- a/drivers/rtc/rtc-pcf85063.c
+++ b/drivers/rtc/rtc-pcf85063.c
@@ -501,15 +501,6 @@ static struct clk *pcf85063_clkout_register_clk(struct 
pcf85063 *pcf85063)
 }
 #endif
 
-static const struct pcf85063_config pcf85063a_config = {
-   .regmap = {
-   .reg_bits = 8,
-   .val_bits = 8,
-   .max_register = 0x11,
-   },
-   .has_alarms = 1,
-};
-
 static const struct pcf85063_config pcf85063tp_config = {
.regmap = {
.reg_bits = 8,
@@ -518,16 +509,6 @@ static const struct pcf85063_config pcf85063tp_config = {
},
 };
 
-static const struct pcf85063_config rv8263_config = {
-   .regmap = {
-   .reg_bits = 8,
-   .val_bits = 8,
-   .max_register = 0x11,
-   },
-   .has_alarms = 1,
-   .force_cap_7000 = 1,
-};
-
 static int pcf85063_probe(struct i2c_client *client)
 {
struct pcf85063 *pcf85063;
@@ -611,6 +592,25 @@ static int pcf85063_probe(struct i2c_client *client)
 }
 
 #ifdef CONFIG_OF
+static const struct pcf85063_config pcf85063a_config = {
+   .regmap = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = 0x11,
+   },
+   .has_alarms = 1,
+};
+
+static const struct pcf85063_config rv8263_config = {
+   .regmap = {
+   .reg_bits = 8,
+   .val_bits = 8,
+   .max_register = 0x11,
+   },
+   .has_alarms = 1,
+   .force_cap_7000 = 1,
+};
+
 static const struct of_device_id pcf85063_of_match[] = {
{ .compatible = "nxp,pcf85063", .data = _config },
{ .compatible = "nxp,pcf85063tp", .data = _config },
-- 
2.29.2



[PATCH 13/21] rtc: rs5c372: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rs5c372.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rs5c372.c b/drivers/rtc/rtc-rs5c372.c
index 3bd6eaa0dcf6..80980414890c 100644
--- a/drivers/rtc/rtc-rs5c372.c
+++ b/drivers/rtc/rtc-rs5c372.c
@@ -83,7 +83,7 @@ static const struct i2c_device_id rs5c372_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rs5c372_id);
 
-static const struct of_device_id rs5c372_of_match[] = {
+static const __maybe_unused struct of_device_id rs5c372_of_match[] = {
{
.compatible = "ricoh,r2025sd",
.data = (void *)rtc_r2025sd
-- 
2.29.2



[PATCH 17/21] rtc: rv8803: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv8803.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv8803.c b/drivers/rtc/rtc-rv8803.c
index 8821264e9385..72adef5a5ebe 100644
--- a/drivers/rtc/rtc-rv8803.c
+++ b/drivers/rtc/rtc-rv8803.c
@@ -607,7 +607,7 @@ static const struct i2c_device_id rv8803_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rv8803_id);
 
-static const struct of_device_id rv8803_of_match[] = {
+static const __maybe_unused struct of_device_id rv8803_of_match[] = {
{
.compatible = "microcrystal,rv8803",
.data = (void *)rv_8803
-- 
2.29.2



[PATCH 14/21] rtc: rv3028: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv3028.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv3028.c b/drivers/rtc/rtc-rv3028.c
index 2ee2afa3fa66..0c48d980d06a 100644
--- a/drivers/rtc/rtc-rv3028.c
+++ b/drivers/rtc/rtc-rv3028.c
@@ -898,7 +898,7 @@ static int rv3028_probe(struct i2c_client *client)
return 0;
 }
 
-static const struct of_device_id rv3028_of_match[] = {
+static const __maybe_unused struct of_device_id rv3028_of_match[] = {
{ .compatible = "microcrystal,rv3028", },
{ }
 };
-- 
2.29.2



[PATCH 21/21] rtc: sd3078: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-sd3078.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-sd3078.c b/drivers/rtc/rtc-sd3078.c
index f6bee69ba017..24e8528e23ec 100644
--- a/drivers/rtc/rtc-sd3078.c
+++ b/drivers/rtc/rtc-sd3078.c
@@ -207,7 +207,7 @@ static const struct i2c_device_id sd3078_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, sd3078_id);
 
-static const struct of_device_id rtc_dt_match[] = {
+static const __maybe_unused struct of_device_id rtc_dt_match[] = {
{ .compatible = "whwave,sd3078" },
{},
 };
-- 
2.29.2



[PATCH 15/21] rtc: rv3029: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv3029c2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv3029c2.c b/drivers/rtc/rtc-rv3029c2.c
index 71102c7fbd7d..8cb84c9595fc 100644
--- a/drivers/rtc/rtc-rv3029c2.c
+++ b/drivers/rtc/rtc-rv3029c2.c
@@ -808,7 +808,7 @@ static const struct i2c_device_id rv3029_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rv3029_id);
 
-static const struct of_device_id rv3029_of_match[] = {
+static const __maybe_unused struct of_device_id rv3029_of_match[] = {
{ .compatible = "microcrystal,rv3029" },
{ }
 };
-- 
2.29.2



[PATCH 16/21] rtc: rv3032: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rv3032.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rv3032.c b/drivers/rtc/rtc-rv3032.c
index 5161016bce00..d63102d5cb1e 100644
--- a/drivers/rtc/rtc-rv3032.c
+++ b/drivers/rtc/rtc-rv3032.c
@@ -906,7 +906,7 @@ static int rv3032_probe(struct i2c_client *client)
return 0;
 }
 
-static const struct of_device_id rv3032_of_match[] = {
+static const __maybe_unused struct of_device_id rv3032_of_match[] = {
{ .compatible = "microcrystal,rv3032", },
{ }
 };
-- 
2.29.2



[PATCH 19/21] rtc: rx8581: quiet maybe-unused variable warning

2021-02-02 Thread Alexandre Belloni
When CONFIG_OF is disabled then the matching table is not referenced.

Signed-off-by: Alexandre Belloni 
---
 drivers/rtc/rtc-rx8581.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-rx8581.c b/drivers/rtc/rtc-rx8581.c
index de109139529b..aed4898a0ff4 100644
--- a/drivers/rtc/rtc-rx8581.c
+++ b/drivers/rtc/rtc-rx8581.c
@@ -314,7 +314,7 @@ static const struct i2c_device_id rx8581_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, rx8581_id);
 
-static const struct of_device_id rx8581_of_match[] = {
+static const __maybe_unused struct of_device_id rx8581_of_match[] = {
{ .compatible = "epson,rx8571", .data = _config },
{ .compatible = "epson,rx8581", .data = _config },
{ /* sentinel */ }
-- 
2.29.2



  1   2   3   4   5   6   7   8   9   10   >