linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver
@ 2019-04-03 13:54 Enric Balletbo i Serra
  2019-04-03 13:54 ` [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver Enric Balletbo i Serra
  2019-04-05 19:54 ` [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Guenter Roeck
  0 siblings, 2 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2019-04-03 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: gwendal, jflat, kernel, lee.jones, bleung, groeck, rajatja, dtor

From: Guenter Roeck <groeck@chromium.org>

The CrOS USB PD logging feature is logically separate functionality of
the charge manager, hence has its own driver. The driver logs the event
data for the USB PD charger available in some ChromeOS Embedded
Controllers.

Signed-off-by: Guenter Roeck <groeck@chromium.org>
[remove APPEND_STRING macro and minor cleanups]
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/Kconfig             |  12 +
 drivers/platform/chrome/Makefile            |   1 +
 drivers/platform/chrome/cros_usbpd_logger.c | 260 ++++++++++++++++++++
 3 files changed, 273 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_usbpd_logger.c

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 9186d81a51cc..4313a8bcf0ab 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -152,6 +152,18 @@ config CROS_EC_SYSFS
 	  To compile this driver as a module, choose M here: the
 	  module will be called cros_ec_sysfs.
 
+config CROS_USBPD_LOGGER
+	tristate "Logging driver for USB PD charger"
+	depends on CHARGER_CROS_USBPD
+	default y
+	select RTC_LIB
+	help
+	  This option enables support for logging event data for the USB PD charger
+	  available in the Embedded Controller on ChromeOS systems.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cros_usbpd_logger.
+
 source "drivers/platform/chrome/wilco_ec/Kconfig"
 
 endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 1e2f0029b597..0bee2e44748e 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_CROS_EC_LIGHTBAR)		+= cros_ec_lightbar.o
 obj-$(CONFIG_CROS_EC_VBC)		+= cros_ec_vbc.o
 obj-$(CONFIG_CROS_EC_DEBUGFS)		+= cros_ec_debugfs.o
 obj-$(CONFIG_CROS_EC_SYSFS)		+= cros_ec_sysfs.o
+obj-$(CONFIG_CROS_USBPD_LOGGER)		+= cros_usbpd_logger.o
 
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
new file mode 100644
index 000000000000..2091ad7c2afa
--- /dev/null
+++ b/drivers/platform/chrome/cros_usbpd_logger.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Logging driver for ChromeOS EC based USBPD Charger.
+ *
+ * Copyright 2018 Google LLC.
+ */
+
+#include <linux/ktime.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#define DRV_NAME "cros-usbpd-logger"
+
+#define CROS_USBPD_MAX_LOG_ENTRIES	30
+#define CROS_USBPD_LOG_UPDATE_DELAY	msecs_to_jiffies(60000)
+#define CROS_USBPD_DATA_SIZE		16
+#define CROS_USBPD_LOG_RESP_SIZE	(sizeof(struct ec_response_pd_log) + \
+					 CROS_USBPD_DATA_SIZE)
+#define CROS_USBPD_BUFFER_SIZE		(sizeof(struct cros_ec_command) + \
+					 CROS_USBPD_LOG_RESP_SIZE)
+/* Buffer for building the PDLOG string */
+#define BUF_SIZE	80
+
+struct logger_data {
+	struct device *dev;
+	struct cros_ec_dev *ec_dev;
+	u8 ec_buffer[CROS_USBPD_BUFFER_SIZE];
+	struct delayed_work log_work;
+	struct workqueue_struct *log_workqueue;
+};
+
+static const char * const chg_type_names[] = {
+	"None", "PD", "Type-C", "Proprietary", "DCP", "CDP", "SDP",
+	"Other", "VBUS"
+};
+
+static const char * const role_names[] = {
+	"Disconnected", "SRC", "SNK", "SNK (not charging)"
+};
+
+static const char * const fault_names[] = {
+	"---", "OCP", "fast OCP", "OVP", "Discharge"
+};
+
+static int append_str(char *buf, int pos, const char *fmt, ...)
+{
+	va_list args;
+	int i;
+
+	va_start(args, fmt);
+	i = vsnprintf(buf + pos, BUF_SIZE - pos, fmt, args);
+	va_end(args);
+
+	return i;
+}
+
+static struct ec_response_pd_log *ec_get_log_entry(struct logger_data *logger)
+{
+	struct cros_ec_dev *ec_dev = logger->ec_dev;
+	struct cros_ec_command *msg;
+	int ret;
+
+	msg = (struct cros_ec_command *)logger->ec_buffer;
+
+	msg->command = ec_dev->cmd_offset + EC_CMD_PD_GET_LOG_ENTRY;
+	msg->insize = CROS_USBPD_LOG_RESP_SIZE;
+
+	ret = cros_ec_cmd_xfer_status(ec_dev->ec_dev, msg);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return (struct ec_response_pd_log *)msg->data;
+}
+
+static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
+				       ktime_t tstamp)
+{
+	const char *fault, *role, *chg_type;
+	struct usb_chg_measures *meas;
+	struct mcdp_info *minfo;
+	int role_idx, type_idx;
+	char buf[BUF_SIZE + 1];
+	struct rtc_time rt;
+	int len = 0;
+	int i;
+
+	/* The timestamp is the number of 1024th of seconds in the past */
+	tstamp = ktime_sub_us(tstamp, r->timestamp << PD_LOG_TIMESTAMP_SHIFT);
+	rt = rtc_ktime_to_tm(tstamp);
+
+	switch (r->type) {
+	case PD_EVENT_MCU_CHARGE:
+		if (r->data & CHARGE_FLAGS_OVERRIDE)
+			len += append_str(buf, len, "override ");
+
+		if (r->data & CHARGE_FLAGS_DELAYED_OVERRIDE)
+			len += append_str(buf, len, "pending_override ");
+
+		role_idx = r->data & CHARGE_FLAGS_ROLE_MASK;
+		role = role_idx < ARRAY_SIZE(role_names) ?
+			role_names[role_idx] : "Unknown";
+
+		type_idx = (r->data & CHARGE_FLAGS_TYPE_MASK)
+			 >> CHARGE_FLAGS_TYPE_SHIFT;
+
+		chg_type = type_idx < ARRAY_SIZE(chg_type_names) ?
+			chg_type_names[type_idx] : "???";
+
+		if (role_idx == USB_PD_PORT_POWER_DISCONNECTED ||
+		    role_idx == USB_PD_PORT_POWER_SOURCE) {
+			len += append_str(buf, len, "%s", role);
+			break;
+		}
+
+		meas = (struct usb_chg_measures *)r->payload;
+		len += append_str(buf, len, "%s %s %s %dmV max %dmV / %dmA",
+				  role,	r->data & CHARGE_FLAGS_DUAL_ROLE ?
+				  "DRP" : "Charger",
+				  chg_type, meas->voltage_now,
+				  meas->voltage_max, meas->current_max);
+		break;
+	case PD_EVENT_ACC_RW_FAIL:
+		len += append_str(buf, len, "RW signature check failed");
+		break;
+	case PD_EVENT_PS_FAULT:
+		fault = r->data < ARRAY_SIZE(fault_names) ? fault_names[r->data]
+							  : "???";
+		len += append_str(buf, len, "Power supply fault: %s", fault);
+		break;
+	case PD_EVENT_VIDEO_DP_MODE:
+		len += append_str(buf, len, "DP mode %sabled", r->data == 1 ?
+				  "en" : "dis");
+		break;
+	case PD_EVENT_VIDEO_CODEC:
+		minfo = (struct mcdp_info *)r->payload;
+		len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x"
+				  MCDP_FAMILY(minfo->family),
+				  MCDP_CHIPID(minfo->chipid));
+		len += append_str(buf, len, " irom:%d.%d.%d fw:%d.%d.%d",
+				  minfo->irom.major, minfo->irom.minor,
+				  minfo->irom.build, minfo->fw.major,
+				  minfo->fw.minor, minfo->fw.build);
+		break;
+	default:
+		len += append_str(buf, len, "Event %02x (%04x) [", r->type,
+				  r->data);
+
+		for (i = 0; i < PD_LOG_SIZE(r->size_port); i++)
+			len += append_str(buf, len, "%02x ", r->payload[i]);
+
+		len += append_str(buf, len, "]");
+		break;
+	}
+
+	pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03lld P%d %s\n",
+		rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
+		rt.tm_hour, rt.tm_min, rt.tm_sec,
+		ktime_to_ms(tstamp) % MSEC_PER_SEC,
+		PD_LOG_PORT(r->size_port), buf);
+}
+
+static void cros_usbpd_log_check(struct work_struct *work)
+{
+	struct logger_data *logger = container_of(to_delayed_work(work),
+						  struct logger_data,
+						  log_work);
+	struct device *dev = logger->dev;
+	struct ec_response_pd_log *r;
+	int entries = 0;
+	ktime_t now;
+
+	while (entries++ < CROS_USBPD_MAX_LOG_ENTRIES) {
+		r = ec_get_log_entry(logger);
+		now = ktime_get_real();
+		if (IS_ERR(r)) {
+			dev_dbg(dev, "Cannot get PD log %ld\n", PTR_ERR(r));
+			break;
+		}
+		if (r->type == PD_EVENT_NO_ENTRY)
+			break;
+
+		cros_usbpd_print_log_entry(r, now);
+	}
+
+	queue_delayed_work(logger->log_workqueue, &logger->log_work,
+			   CROS_USBPD_LOG_UPDATE_DELAY);
+}
+
+static int cros_usbpd_logger_probe(struct platform_device *pd)
+{
+	struct cros_ec_dev *ec_dev = dev_get_drvdata(pd->dev.parent);
+	struct device *dev = &pd->dev;
+	struct logger_data *logger;
+
+	logger = devm_kzalloc(dev, sizeof(*logger), GFP_KERNEL);
+	if (!logger)
+		return -ENOMEM;
+
+	logger->dev = dev;
+	logger->ec_dev = ec_dev;
+
+	platform_set_drvdata(pd, logger);
+
+	/* Retrieve PD event logs periodically */
+	INIT_DELAYED_WORK(&logger->log_work, cros_usbpd_log_check);
+	logger->log_workqueue =	create_singlethread_workqueue("cros_usbpd_log");
+	queue_delayed_work(logger->log_workqueue, &logger->log_work,
+			   CROS_USBPD_LOG_UPDATE_DELAY);
+
+	return 0;
+}
+
+static int cros_usbpd_logger_remove(struct platform_device *pd)
+{
+	struct logger_data *logger = platform_get_drvdata(pd);
+
+	cancel_delayed_work_sync(&logger->log_work);
+
+	return 0;
+}
+
+static int __maybe_unused cros_usbpd_logger_resume(struct device *dev)
+{
+	struct logger_data *logger = dev_get_drvdata(dev);
+
+	queue_delayed_work(logger->log_workqueue, &logger->log_work,
+			   CROS_USBPD_LOG_UPDATE_DELAY);
+
+	return 0;
+}
+
+static int __maybe_unused cros_usbpd_logger_suspend(struct device *dev)
+{
+	struct logger_data *logger = dev_get_drvdata(dev);
+
+	cancel_delayed_work_sync(&logger->log_work);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cros_usbpd_logger_pm_ops, cros_usbpd_logger_suspend,
+			 cros_usbpd_logger_resume);
+
+static struct platform_driver cros_usbpd_logger_driver = {
+	.driver = {
+		.name = DRV_NAME,
+		.pm = &cros_usbpd_logger_pm_ops,
+	},
+	.probe = cros_usbpd_logger_probe,
+	.remove = cros_usbpd_logger_remove,
+};
+
+module_platform_driver(cros_usbpd_logger_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Logging driver for ChromeOS EC USBPD Charger.");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver
  2019-04-03 13:54 [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Enric Balletbo i Serra
@ 2019-04-03 13:54 ` Enric Balletbo i Serra
  2019-04-03 14:03   ` Enric Balletbo i Serra
  2019-04-05 19:54 ` [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Guenter Roeck
  1 sibling, 1 reply; 7+ messages in thread
From: Enric Balletbo i Serra @ 2019-04-03 13:54 UTC (permalink / raw)
  To: linux-kernel
  Cc: gwendal, jflat, kernel, lee.jones, bleung, groeck, rajatja, dtor

Add the cros-usbpd-logger driver for logging event data for the USB PD
charger available in the Embedded Controller on ChromeOS systems. The
logging feature is logically separate functionality from charge manager,
hence is instantiated as a different driver.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Lee, this is send as separate patch because touches the MFD subsystem,
apart from that the driver won't be instantiated if [1/2] is not
merged, which I think is not a problem, it is safe for both go through
their subsystem separately.

Thanks,
 Enric

 drivers/mfd/cros_ec_dev.c                   | 3 ++-
 drivers/platform/chrome/cros_usbpd_logger.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index 0638a0d82d97..3e33fa5b8657 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -612,7 +612,8 @@ static const struct mfd_cell cros_ec_rtc_cells[] = {
 };
 
 static const struct mfd_cell cros_usbpd_charger_cells[] = {
-	{ .name = "cros-usbpd-charger" }
+	{ .name = "cros-usbpd-charger" },
+	{ .name = "cros-usbpd-logger" },
 };
 
 static const struct mfd_cell cros_ec_platform_cells[] = {
diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
index 2091ad7c2afa..eb27078aae47 100644
--- a/drivers/platform/chrome/cros_usbpd_logger.c
+++ b/drivers/platform/chrome/cros_usbpd_logger.c
@@ -136,10 +136,10 @@ static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
 		break;
 	case PD_EVENT_VIDEO_CODEC:
 		minfo = (struct mcdp_info *)r->payload;
-		len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x"
+		len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x ",
 				  MCDP_FAMILY(minfo->family),
 				  MCDP_CHIPID(minfo->chipid));
-		len += append_str(buf, len, " irom:%d.%d.%d fw:%d.%d.%d",
+		len += append_str(buf, len, "irom:%d.%d.%d fw:%d.%d.%d",
 				  minfo->irom.major, minfo->irom.minor,
 				  minfo->irom.build, minfo->fw.major,
 				  minfo->fw.minor, minfo->fw.build);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver
  2019-04-03 13:54 ` [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver Enric Balletbo i Serra
@ 2019-04-03 14:03   ` Enric Balletbo i Serra
  0 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo i Serra @ 2019-04-03 14:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: gwendal, jflat, kernel, lee.jones, bleung, groeck, rajatja, dtor



On 3/4/19 15:54, Enric Balletbo i Serra wrote:
> Add the cros-usbpd-logger driver for logging event data for the USB PD
> charger available in the Embedded Controller on ChromeOS systems. The
> logging feature is logically separate functionality from charge manager,
> hence is instantiated as a different driver.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> Lee, this is send as separate patch because touches the MFD subsystem,
> apart from that the driver won't be instantiated if [1/2] is not
> merged, which I think is not a problem, it is safe for both go through
> their subsystem separately.
> 
> Thanks,
>  Enric
> 
>  drivers/mfd/cros_ec_dev.c                   | 3 ++-
>  drivers/platform/chrome/cros_usbpd_logger.c | 4 ++--

Uggh, sending a v2, sorry for the noise.

>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index 0638a0d82d97..3e33fa5b8657 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -612,7 +612,8 @@ static const struct mfd_cell cros_ec_rtc_cells[] = {
>  };
>  
>  static const struct mfd_cell cros_usbpd_charger_cells[] = {
> -	{ .name = "cros-usbpd-charger" }
> +	{ .name = "cros-usbpd-charger" },
> +	{ .name = "cros-usbpd-logger" },
>  };
>  
>  static const struct mfd_cell cros_ec_platform_cells[] = {
> diff --git a/drivers/platform/chrome/cros_usbpd_logger.c b/drivers/platform/chrome/cros_usbpd_logger.c
> index 2091ad7c2afa..eb27078aae47 100644
> --- a/drivers/platform/chrome/cros_usbpd_logger.c
> +++ b/drivers/platform/chrome/cros_usbpd_logger.c
> @@ -136,10 +136,10 @@ static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
>  		break;
>  	case PD_EVENT_VIDEO_CODEC:
>  		minfo = (struct mcdp_info *)r->payload;
> -		len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x"
> +		len += append_str(buf, len, "HDMI info: family:%04x chipid:%04x ",
>  				  MCDP_FAMILY(minfo->family),
>  				  MCDP_CHIPID(minfo->chipid));
> -		len += append_str(buf, len, " irom:%d.%d.%d fw:%d.%d.%d",
> +		len += append_str(buf, len, "irom:%d.%d.%d fw:%d.%d.%d",
>  				  minfo->irom.major, minfo->irom.minor,
>  				  minfo->irom.build, minfo->fw.major,
>  				  minfo->fw.minor, minfo->fw.build);
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver
  2019-04-03 13:54 [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Enric Balletbo i Serra
  2019-04-03 13:54 ` [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver Enric Balletbo i Serra
@ 2019-04-05 19:54 ` Guenter Roeck
  2019-04-05 21:09   ` Enric Balletbo Serra
  1 sibling, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2019-04-05 19:54 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: linux-kernel, Gwendal Grignou, jflat, kernel, Lee Jones,
	Benson Leung, Guenter Roeck, Rajat Jain, Dmitry Torokhov

Hi Enric,

On Wed, Apr 3, 2019 at 6:54 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> From: Guenter Roeck <groeck@chromium.org>
>
> The CrOS USB PD logging feature is logically separate functionality of
> the charge manager, hence has its own driver. The driver logs the event
> data for the USB PD charger available in some ChromeOS Embedded
> Controllers.
>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>
> [remove APPEND_STRING macro and minor cleanups]
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
[ ... ]

> +
> +static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
> +                                      ktime_t tstamp)
> +{
[ ... ]
> +
> +       pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03lld P%d %s\n",
> +               rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
> +               rt.tm_hour, rt.tm_min, rt.tm_sec,
> +               ktime_to_ms(tstamp) % MSEC_PER_SEC,

0day rightfully complains that this introduces a 64-bit divide
operation. I don't know if do_div() works here since it tries to
modify the first parameter. Maybe we need an interim variable to store
ktime_to_ms(tstamp). Something like

    s64 temp = ktime_to_ms(tstamp);
    ...
                    do_div(temp, MSEC_PER_SEC),

Guenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver
  2019-04-05 19:54 ` [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Guenter Roeck
@ 2019-04-05 21:09   ` Enric Balletbo Serra
  2019-04-05 21:38     ` Guenter Roeck
  0 siblings, 1 reply; 7+ messages in thread
From: Enric Balletbo Serra @ 2019-04-05 21:09 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Enric Balletbo i Serra, linux-kernel, Gwendal Grignou, jflat,
	kernel, Lee Jones, Benson Leung, Guenter Roeck, Rajat Jain,
	Dmitry Torokhov

Hi Guenter,

Missatge de Guenter Roeck <groeck@google.com> del dia dv., 5 d’abr.
2019 a les 21:59:
>
> Hi Enric,
>
> On Wed, Apr 3, 2019 at 6:54 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > From: Guenter Roeck <groeck@chromium.org>
> >
> > The CrOS USB PD logging feature is logically separate functionality of
> > the charge manager, hence has its own driver. The driver logs the event
> > data for the USB PD charger available in some ChromeOS Embedded
> > Controllers.
> >
> > Signed-off-by: Guenter Roeck <groeck@chromium.org>
> > [remove APPEND_STRING macro and minor cleanups]
> > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > ---
> >
> [ ... ]
>
> > +
> > +static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
> > +                                      ktime_t tstamp)
> > +{
> [ ... ]
> > +
> > +       pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03lld P%d %s\n",
> > +               rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
> > +               rt.tm_hour, rt.tm_min, rt.tm_sec,
> > +               ktime_to_ms(tstamp) % MSEC_PER_SEC,
>
> 0day rightfully complains that this introduces a 64-bit divide
> operation. I don't know if do_div() works here since it tries to
> modify the first parameter. Maybe we need an interim variable to store
> ktime_to_ms(tstamp). Something like
>
>     s64 temp = ktime_to_ms(tstamp);
>     ...
>                     do_div(temp, MSEC_PER_SEC),
>

I did something similar this afternoon and pushed again for the auto
builders to play with.

+ div_s64_rem(ktime_to_ms(tstamp), MSEC_PER_SEC, &rem);

Unfortunately, 0day now reports another error

drivers/platform/chrome/cros_usbpd_logger.o: In function
`cros_usbpd_print_log_entry':
>> cros_usbpd_logger.c:(.text+0x5a0): undefined reference to `__aeabi_ldivmod'

I'll investigate in more detail Monday morning.

Thanks
- Enric

> Guenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver
  2019-04-05 21:09   ` Enric Balletbo Serra
@ 2019-04-05 21:38     ` Guenter Roeck
  2019-04-05 21:43       ` Enric Balletbo Serra
  0 siblings, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2019-04-05 21:38 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: Enric Balletbo i Serra, linux-kernel, Gwendal Grignou, jflat,
	kernel, Lee Jones, Benson Leung, Guenter Roeck, Rajat Jain,
	Dmitry Torokhov

On Fri, Apr 5, 2019 at 2:09 PM Enric Balletbo Serra <eballetbo@gmail.com> wrote:
>
> Hi Guenter,
>
> Missatge de Guenter Roeck <groeck@google.com> del dia dv., 5 d’abr.
> 2019 a les 21:59:
> >
> > Hi Enric,
> >
> > On Wed, Apr 3, 2019 at 6:54 AM Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> > >
> > > From: Guenter Roeck <groeck@chromium.org>
> > >
> > > The CrOS USB PD logging feature is logically separate functionality of
> > > the charge manager, hence has its own driver. The driver logs the event
> > > data for the USB PD charger available in some ChromeOS Embedded
> > > Controllers.
> > >
> > > Signed-off-by: Guenter Roeck <groeck@chromium.org>
> > > [remove APPEND_STRING macro and minor cleanups]
> > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > ---
> > >
> > [ ... ]
> >
> > > +
> > > +static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
> > > +                                      ktime_t tstamp)
> > > +{
> > [ ... ]
> > > +
> > > +       pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03lld P%d %s\n",
> > > +               rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
> > > +               rt.tm_hour, rt.tm_min, rt.tm_sec,
> > > +               ktime_to_ms(tstamp) % MSEC_PER_SEC,
> >
> > 0day rightfully complains that this introduces a 64-bit divide
> > operation. I don't know if do_div() works here since it tries to
> > modify the first parameter. Maybe we need an interim variable to store
> > ktime_to_ms(tstamp). Something like
> >
> >     s64 temp = ktime_to_ms(tstamp);
> >     ...
> >                     do_div(temp, MSEC_PER_SEC),
> >
>
> I did something similar this afternoon and pushed again for the auto
> builders to play with.
>
> + div_s64_rem(ktime_to_ms(tstamp), MSEC_PER_SEC, &rem);
>
> Unfortunately, 0day now reports another error
>
> drivers/platform/chrome/cros_usbpd_logger.o: In function
> `cros_usbpd_print_log_entry':
> >> cros_usbpd_logger.c:(.text+0x5a0): undefined reference to `__aeabi_ldivmod'
>

I think that may have been with the earlier version of the code. Your
top of tree doesn't include the SHA that is reported as failing.

Guenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver
  2019-04-05 21:38     ` Guenter Roeck
@ 2019-04-05 21:43       ` Enric Balletbo Serra
  0 siblings, 0 replies; 7+ messages in thread
From: Enric Balletbo Serra @ 2019-04-05 21:43 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Enric Balletbo i Serra, linux-kernel, Gwendal Grignou, jflat,
	kernel, Lee Jones, Benson Leung, Guenter Roeck, Rajat Jain,
	Dmitry Torokhov

Missatge de Guenter Roeck <groeck@google.com> del dia dv., 5 d’abr.
2019 a les 23:38:
>
> On Fri, Apr 5, 2019 at 2:09 PM Enric Balletbo Serra <eballetbo@gmail.com> wrote:
> >
> > Hi Guenter,
> >
> > Missatge de Guenter Roeck <groeck@google.com> del dia dv., 5 d’abr.
> > 2019 a les 21:59:
> > >
> > > Hi Enric,
> > >
> > > On Wed, Apr 3, 2019 at 6:54 AM Enric Balletbo i Serra
> > > <enric.balletbo@collabora.com> wrote:
> > > >
> > > > From: Guenter Roeck <groeck@chromium.org>
> > > >
> > > > The CrOS USB PD logging feature is logically separate functionality of
> > > > the charge manager, hence has its own driver. The driver logs the event
> > > > data for the USB PD charger available in some ChromeOS Embedded
> > > > Controllers.
> > > >
> > > > Signed-off-by: Guenter Roeck <groeck@chromium.org>
> > > > [remove APPEND_STRING macro and minor cleanups]
> > > > Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > > ---
> > > >
> > > [ ... ]
> > >
> > > > +
> > > > +static void cros_usbpd_print_log_entry(struct ec_response_pd_log *r,
> > > > +                                      ktime_t tstamp)
> > > > +{
> > > [ ... ]
> > > > +
> > > > +       pr_info("PDLOG %d/%02d/%02d %02d:%02d:%02d.%03lld P%d %s\n",
> > > > +               rt.tm_year + 1900, rt.tm_mon + 1, rt.tm_mday,
> > > > +               rt.tm_hour, rt.tm_min, rt.tm_sec,
> > > > +               ktime_to_ms(tstamp) % MSEC_PER_SEC,
> > >
> > > 0day rightfully complains that this introduces a 64-bit divide
> > > operation. I don't know if do_div() works here since it tries to
> > > modify the first parameter. Maybe we need an interim variable to store
> > > ktime_to_ms(tstamp). Something like
> > >
> > >     s64 temp = ktime_to_ms(tstamp);
> > >     ...
> > >                     do_div(temp, MSEC_PER_SEC),
> > >
> >
> > I did something similar this afternoon and pushed again for the auto
> > builders to play with.
> >
> > + div_s64_rem(ktime_to_ms(tstamp), MSEC_PER_SEC, &rem);
> >
> > Unfortunately, 0day now reports another error
> >
> > drivers/platform/chrome/cros_usbpd_logger.o: In function
> > `cros_usbpd_print_log_entry':
> > >> cros_usbpd_logger.c:(.text+0x5a0): undefined reference to `__aeabi_ldivmod'
> >
>
> I think that may have been with the earlier version of the code. Your
> top of tree doesn't include the SHA that is reported as failing.
>

Oh right, good, thank you for telling me, I guess is time to go to bed
for me :-)

Enric

> Guenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-04-05 21:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-03 13:54 [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Enric Balletbo i Serra
2019-04-03 13:54 ` [PATCH 2/2] mfd: cros_ec: Instantiate the CrOS USB PD logger driver Enric Balletbo i Serra
2019-04-03 14:03   ` Enric Balletbo i Serra
2019-04-05 19:54 ` [PATCH 1/2] platform/chrome: Add CrOS USB PD logging driver Guenter Roeck
2019-04-05 21:09   ` Enric Balletbo Serra
2019-04-05 21:38     ` Guenter Roeck
2019-04-05 21:43       ` Enric Balletbo Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).