* [PATCH v2 0/2] Add RTC driver for Sunplus SP7021 SoC @ 2021-11-09 6:38 Vincent Shih 2021-11-09 6:38 ` [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 Vincent Shih 2021-11-09 6:38 ` [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema Vincent Shih 0 siblings, 2 replies; 6+ messages in thread From: Vincent Shih @ 2021-11-09 6:38 UTC (permalink / raw) To: a.zummo, alexandre.belloni, p.zabel, linux-kernel, linux-rtc, robh+dt, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih Cc: Vincent Shih This is a patch series for RTC driver for Sunplus SP7021 SoC. Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD Card and etc.) into a single chip. It is designed for industrial control. Refer to: https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview https://tibbo.com/store/plus1.html Vincent Shih (2): rtc: Add driver for Sunplus SP7021 dt-bindings: rtc: Convert Sunplus RTC to json-schema .../bindings/rtc/sunplus,sp7021-rtc.yaml | 56 ++++ MAINTAINERS | 7 + drivers/rtc/Kconfig | 11 + drivers/rtc/Makefile | 1 + drivers/rtc/rtc-sunplus.c | 370 +++++++++++++++++++++ 5 files changed, 445 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml create mode 100644 drivers/rtc/rtc-sunplus.c -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 2021-11-09 6:38 [PATCH v2 0/2] Add RTC driver for Sunplus SP7021 SoC Vincent Shih @ 2021-11-09 6:38 ` Vincent Shih 2021-11-29 9:45 ` Philipp Zabel 2021-11-29 11:20 ` Alexandre Belloni 2021-11-09 6:38 ` [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema Vincent Shih 1 sibling, 2 replies; 6+ messages in thread From: Vincent Shih @ 2021-11-09 6:38 UTC (permalink / raw) To: a.zummo, alexandre.belloni, p.zabel, linux-kernel, linux-rtc, robh+dt, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih Cc: Vincent Shih Add driver for Sunplus SP7021 Signed-off-by: Vincent Shih <vincent.shih@sunplus.com> --- Changes in v2: - Addressed the comments from Mr. Philipp Zabel - Addressed the comments from Mr. Randy Dunlap - Addressed the comments from Mr. Alexandre Belloni - Sorted the files in Makefile MAINTAINERS | 6 + drivers/rtc/Kconfig | 11 ++ drivers/rtc/Makefile | 1 + drivers/rtc/rtc-sunplus.c | 370 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 388 insertions(+) create mode 100644 drivers/rtc/rtc-sunplus.c diff --git a/MAINTAINERS b/MAINTAINERS index 3b79fd4..6c1a535 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17945,6 +17945,12 @@ L: netdev@vger.kernel.org S: Maintained F: drivers/net/ethernet/dlink/sundance.c +SUNPLUS RTC DRIVER +M: Vincent Shih <vincent.shih@sunplus.com> +L: linux-rtc@vger.kernel.org +S: Maintained +F: drivers/rtc/rtc-sunplus.c + SUPERH M: Yoshinori Sato <ysato@users.sourceforge.jp> M: Rich Felker <dalias@libc.org> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index e1bc521..7ca6765 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -1028,6 +1028,17 @@ config RTC_DRV_DS1685_FAMILY This driver can also be built as a module. If so, the module will be called rtc-ds1685. +config RTC_DRV_SUNPLUS + bool "Sunplus SP7021 RTC" + depends on SOC_SP7021 + help + Say 'yes' to get support for Sunplus SP7021 real-time clock + (RTC) for industrial applications. + + It provides RTC status check, timer/alarm functionalities, + user data reservation only with battery with voltage over 2.5V, + RTC power status check and battery charge. + choice prompt "Subtype" depends on RTC_DRV_DS1685_FAMILY diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index 5ceeafe..93cb4b2 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -164,6 +164,7 @@ obj-$(CONFIG_RTC_DRV_STM32) += rtc-stm32.o obj-$(CONFIG_RTC_DRV_STMP) += rtc-stmp3xxx.o obj-$(CONFIG_RTC_DRV_SUN4V) += rtc-sun4v.o obj-$(CONFIG_RTC_DRV_SUN6I) += rtc-sun6i.o +obj-$(CONFIG_RTC_DRV_SUNPLUS) += rtc-sunplus.o obj-$(CONFIG_RTC_DRV_SUNXI) += rtc-sunxi.o obj-$(CONFIG_RTC_DRV_TEGRA) += rtc-tegra.o obj-$(CONFIG_RTC_DRV_TEST) += rtc-test.o diff --git a/drivers/rtc/rtc-sunplus.c b/drivers/rtc/rtc-sunplus.c new file mode 100644 index 0000000..73c400d --- /dev/null +++ b/drivers/rtc/rtc-sunplus.c @@ -0,0 +1,370 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* */ +/* The RTC driver for Sunplus SP7021 */ +/* */ +/* Copyright (C) 2019 Sunplus Technology Inc., All rights reseerved. */ +/* */ + +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/io.h> +#include <linux/ktime.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/reset.h> +#include <linux/rtc.h> +#include <linux/platform_device.h> + +#define RTC_REG_NAME "rtc_reg" + +#define RTC_CTRL 0x40 +#define TIMER_FREEZE (0x1 << 5) +#define TIMER_FREEZE_MASK (1 << (5 + 16)) +#define DIS_SYS_RST_RTC (0x1 << 4) +#define DIS_SYS_RST_RTC_MASK (1 << (4 + 16)) +#define RTC32K_MODE_RESET (0x1 << 3) +#define RTC32K_MODE_RESET_MASK (1 << (3 + 16)) +#define ALARM_EN_OVERDUE (0x1 << 2) +#define ALARM_EN_OVERDUE_MASK (1 << (2 + 16)) +#define ALARM_EN_PMC (0x1 << 1) +#define ALARM_EN_PMC_MASK (1 << (1 + 16)) +#define ALARM_EN (0x1 << 0) +#define ALARM_EN_MASK (1 << (0 + 16)) +#define RTC_TIMER_OUT 0x44 +#define RTC_DIVIDER 0x48 +#define RTC_TIMER_SET 0x4c +#define RTC_ALARM_SET 0x50 +#define RTC_USER_DATA 0x54 +#define RTC_RESET_RECORD 0x58 +#define RTC_BATTERY_CTRL 0x5c +#define BAT_CHARGE_RSEL_2K_OHM (0x0 << 2) +#define BAT_CHARGE_RSEL_250_OHM (0x1 << 2) +#define BAT_CHARGE_RSEL_50_OHM (0x2 << 2) +#define BAT_CHARGE_RSEL_0_OHM (0x3 << 2) +#define BAT_CHARGE_RSEL_MASK (0x3 << (2 + 16)) +#define BAT_CHARGE_DSEL_ON (0x0 << 1) +#define BAT_CHARGE_DSEL_OFF (0x1 << 1) +#define BAT_CHARGE_DSEL_MASK (0x1 << (1 + 16)) +#define BAT_CHARGE_EN (0x1 << 0) +#define BAT_CHARGE_EN_MASK (0x1 << (0 + 16)) +#define RTC_TRIM_CTRL 0x60 + +struct sunplus_rtc { + struct clk *rtcclk; + struct reset_control *rstc; + void __iomem *base; + u32 ohms; +}; + +struct sunplus_rtc sp_rtc; + +static void sp_get_seconds(unsigned long *secs) +{ + *secs = (unsigned long)readl(sp_rtc.base + RTC_TIMER_OUT); +} + +static void sp_set_seconds(unsigned long secs) +{ + writel((u32)secs, sp_rtc.base + RTC_TIMER_SET); +} + +static int sp_rtc_read_time(struct device *dev, struct rtc_time *tm) +{ + unsigned long secs; + + sp_get_seconds(&secs); + rtc_time64_to_tm(secs, tm); + dev_dbg(dev, "%s: RTC date/time to %d-%d-%d, %02d:%02d:%02d.\r\n", + __func__, tm->tm_mday, tm->tm_mon + 1, tm->tm_year, + tm->tm_hour, tm->tm_min, tm->tm_sec); + + return rtc_valid_tm(tm); +} + +int sp_rtc_get_time(struct rtc_time *tm) +{ + unsigned long secs; + + sp_get_seconds(&secs); + rtc_time64_to_tm(secs, tm); + + return 0; +} + +static int sp_rtc_suspend(struct platform_device *pdev, pm_message_t state) +{ + dev_dbg(&pdev->dev, "[RTC] Debug: %s(%d)\n", __func__, __LINE__); + + // Keep RTC from system reset + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); + + return 0; +} + +static int sp_rtc_resume(struct platform_device *pdev) +{ + /* */ + /* Because RTC is still powered during suspend, */ + /* there is nothing to do here. */ + /* */ + dev_dbg(&pdev->dev, "[RTC] Debug: %s(%d)\n", __func__, __LINE__); + + // Keep RTC from system reset + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); + + return 0; +} + +static int sp_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + unsigned long secs; + + secs = rtc_tm_to_time64(tm); + dev_dbg(dev, "%s, secs = %lu\n", __func__, secs); + sp_set_seconds(secs); + + return 0; +} + +static int sp_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) +{ + unsigned long alarm_time; + + alarm_time = rtc_tm_to_time64(&alrm->time); + dev_dbg(dev, "%s, alarm_time: %u\n", __func__, (u32)(alarm_time)); + writel((u32)alarm_time, sp_rtc.base + RTC_ALARM_SET); + + return 0; +} + +static int sp_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) +{ + unsigned int alarm_time; + + alarm_time = readl(sp_rtc.base + RTC_ALARM_SET); + dev_dbg(dev, "%s, alarm_time: %u\n", __func__, alarm_time); + + if (alarm_time == 0) { + // alarm is disabled + alrm->enabled = 0; + } else { + // alarm is enabled + alrm->enabled = 1; + rtc_time64_to_tm((unsigned long)(alarm_time), &alrm->time); + } + + return 0; +} + +static int sp_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) +{ + if (enabled) + writel((TIMER_FREEZE_MASK | DIS_SYS_RST_RTC_MASK | RTC32K_MODE_RESET_MASK | + ALARM_EN_OVERDUE_MASK | ALARM_EN_PMC_MASK | ALARM_EN_MASK) | + (DIS_SYS_RST_RTC | ALARM_EN_OVERDUE | ALARM_EN_PMC | + ALARM_EN), sp_rtc.base + RTC_CTRL); + else + writel((ALARM_EN_OVERDUE_MASK | ALARM_EN_PMC_MASK | ALARM_EN_MASK) | 0x0, + sp_rtc.base + RTC_CTRL); + + return 0; +} + +static const struct rtc_class_ops sp_rtc_ops = { + .read_time = sp_rtc_read_time, + .set_time = sp_rtc_set_time, + .set_alarm = sp_rtc_set_alarm, + .read_alarm = sp_rtc_read_alarm, + .alarm_irq_enable = sp_rtc_alarm_irq_enable, +}; + +static irqreturn_t rtc_irq_handler(int irq, void *dev_id) +{ + struct platform_device *plat_dev = dev_id; + struct rtc_device *rtc = platform_get_drvdata(plat_dev); + + rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF); + dev_dbg(&plat_dev->dev, "[RTC] ALARM INT\n"); + + return IRQ_HANDLED; +} + +/* ---------------------------------------------------------------------------------------------- */ +/* bat_charge_rsel bat_charge_dsel bat_charge_en Remarks */ +/* x x 0 Disable */ +/* 0 0 1 0.86mA (2K Ohm with diode) */ +/* 1 0 1 1.81mA (250 Ohm with diode) */ +/* 2 0 1 2.07mA (50 Ohm with diode) */ +/* 3 0 1 16.0mA (0 Ohm with diode) */ +/* 0 1 1 1.36mA (2K Ohm without diode) */ +/* 1 1 1 3.99mA (250 Ohm without diode) */ +/* 2 1 1 4.41mA (50 Ohm without diode) */ +/* 3 1 1 16.0mA (0 Ohm without diode) */ +/* ---------------------------------------------------------------------------------------------- */ +static void sp_rtc_set_trickle_charger(struct device dev) +{ + int ret; + u32 rsel; + + ret = of_property_read_u32(dev.of_node, "trickle-resistor-ohms", &sp_rtc.ohms); + if (ret) + return; + + switch (sp_rtc.ohms) { + case 2000: + rsel = BAT_CHARGE_RSEL_2K_OHM; + break; + case 250: + rsel = BAT_CHARGE_RSEL_250_OHM; + break; + case 50: + rsel = BAT_CHARGE_RSEL_50_OHM; + break; + case 0: + rsel = BAT_CHARGE_RSEL_0_OHM; + break; + default: + return; + } + + writel(BAT_CHARGE_RSEL_MASK | rsel, sp_rtc.base + RTC_BATTERY_CTRL); + + ret = of_property_read_bool(dev.of_node, "trickle-diode-disable"); + if (ret) + writel(BAT_CHARGE_DSEL_MASK | BAT_CHARGE_DSEL_OFF, sp_rtc.base + RTC_BATTERY_CTRL); + else + writel(BAT_CHARGE_DSEL_MASK | BAT_CHARGE_DSEL_ON, sp_rtc.base + RTC_BATTERY_CTRL); + + writel(BAT_CHARGE_EN_MASK | BAT_CHARGE_EN, sp_rtc.base + RTC_BATTERY_CTRL); +} + +static int sp_rtc_probe(struct platform_device *plat_dev) +{ + struct rtc_device *rtc; + struct resource *res; + const struct rtc_class_ops *rtc_ops; + void __iomem *reg_base; + int ret, irq; + + memset(&sp_rtc, 0, sizeof(sp_rtc)); + + // find and map our resources + res = platform_get_resource_byname(plat_dev, IORESOURCE_MEM, RTC_REG_NAME); + + if (res) { + dev_dbg(&plat_dev->dev, "res = 0x%x\n", res->start); + + reg_base = devm_ioremap_resource(&plat_dev->dev, res); + if (IS_ERR(reg_base)) { + dev_err(&plat_dev->dev, "%s devm_ioremap_resource fail\n", RTC_REG_NAME); + return PTR_ERR(reg_base); + } + + dev_dbg(&plat_dev->dev, "reg_base = 0x%lx\n", (unsigned long)reg_base); + } + + sp_rtc.base = reg_base; + rtc_ops = &sp_rtc_ops; + + // Keep RTC from system reset + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); + + // request irq + irq = platform_get_irq(plat_dev, 0); + if (irq < 0) { + dev_err(&plat_dev->dev, "platform_get_irq failed\n"); + irq = IRQ_NOTCONNECTED; + } + + ret = devm_request_irq(&plat_dev->dev, irq, rtc_irq_handler, + IRQF_TRIGGER_RISING, "rtc irq", plat_dev); + if (ret) { + dev_err(&plat_dev->dev, "devm_request_irq failed: %d\n", ret); + return ret; + } + + // Setup trickle charger + if (plat_dev->dev.of_node) + sp_rtc_set_trickle_charger(plat_dev->dev); + + // reset + sp_rtc.rstc = devm_reset_control_get_exclusive(&plat_dev->dev, NULL); + if (IS_ERR(sp_rtc.rstc)) { + ret = dev_err_probe(&plat_dev->dev, PTR_ERR(sp_rtc.rstc), + "failed to retrieve reset controller\n"); + return PTR_ERR(sp_rtc.rstc); + } + + // clk + sp_rtc.rtcclk = devm_clk_get(&plat_dev->dev, NULL); + if (IS_ERR(sp_rtc.rtcclk)) { + dev_err(&plat_dev->dev, "devm_clk_get fail\n"); + return PTR_ERR(sp_rtc.rtcclk); + } + + ret = reset_control_deassert(sp_rtc.rstc); + if (ret) + return ret; + + ret = clk_prepare_enable(sp_rtc.rtcclk); + if (ret) + return ret; + + device_init_wakeup(&plat_dev->dev, 1); + + rtc = devm_rtc_allocate_device(&plat_dev->dev); + if (IS_ERR(rtc)) + return PTR_ERR(rtc); + + rtc->range_max = U32_MAX; + rtc->range_min = 0; + rtc->ops = rtc_ops; + + ret = devm_rtc_register_device(rtc); + if (ret) + goto free_reset_assert_clk; + + platform_set_drvdata(plat_dev, rtc); + dev_info(&plat_dev->dev, "sp7021-rtc loaded\n"); + + return 0; + +free_reset_assert_clk: + reset_control_assert(sp_rtc.rstc); + clk_disable_unprepare(sp_rtc.rtcclk); + + return ret; +} + +static int sp_rtc_remove(struct platform_device *plat_dev) +{ + reset_control_assert(sp_rtc.rstc); + clk_disable_unprepare(sp_rtc.rtcclk); + + return 0; +} + +static const struct of_device_id sp_rtc_of_match[] = { + { .compatible = "sunplus,sp7021-rtc" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, sp_rtc_of_match); + +static struct platform_driver sp_rtc_driver = { + .probe = sp_rtc_probe, + .remove = sp_rtc_remove, + .suspend = sp_rtc_suspend, + .resume = sp_rtc_resume, + .driver = { + .name = "sp7021-rtc", + .owner = THIS_MODULE, + .of_match_table = sp_rtc_of_match, + }, +}; +module_platform_driver(sp_rtc_driver); + +MODULE_AUTHOR("Vincent Shih <vincent.shih@sunplus.com>"); +MODULE_DESCRIPTION("Sunplus RTC driver"); +MODULE_LICENSE("GPL v2"); + -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 2021-11-09 6:38 ` [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 Vincent Shih @ 2021-11-29 9:45 ` Philipp Zabel 2021-11-29 11:20 ` Alexandre Belloni 1 sibling, 0 replies; 6+ messages in thread From: Philipp Zabel @ 2021-11-29 9:45 UTC (permalink / raw) To: Vincent Shih, a.zummo, alexandre.belloni, linux-kernel, linux-rtc, robh+dt, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih Cc: Vincent Shih Hi Vincent, On Tue, 2021-11-09 at 14:38 +0800, Vincent Shih wrote: [...] > +struct sunplus_rtc { > + struct clk *rtcclk; > + struct reset_control *rstc; > + void __iomem *base; > + u32 ohms; > +}; > + > +struct sunplus_rtc sp_rtc; Why is this global? [...] > +static int sp_rtc_probe(struct platform_device *plat_dev) > +{ > + struct rtc_device *rtc; > + struct resource *res; > + const struct rtc_class_ops *rtc_ops; > + void __iomem *reg_base; > + int ret, irq; > + > + memset(&sp_rtc, 0, sizeof(sp_rtc)); I'd allocate device private data here with devm_kzalloc() instead. > + // find and map our resources > + res = platform_get_resource_byname(plat_dev, IORESOURCE_MEM, RTC_REG_NAME); > + > + if (res) { > + dev_dbg(&plat_dev->dev, "res = 0x%x\n", res->start); > + > + reg_base = devm_ioremap_resource(&plat_dev->dev, res); There is no need to check res before feeding it into devm_ioremap_resource(). You can simplify this even further with devm_platform_ioremap_resource_by_name(). > + if (IS_ERR(reg_base)) { > + dev_err(&plat_dev->dev, "%s devm_ioremap_resource fail\n", RTC_REG_NAME); > + return PTR_ERR(reg_base); > + } > + > + dev_dbg(&plat_dev->dev, "reg_base = 0x%lx\n", (unsigned long)reg_base); > + } > + > + sp_rtc.base = reg_base; > + rtc_ops = &sp_rtc_ops; > + > + // Keep RTC from system reset > + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); Are you allowed to write to sp_rtc.base registers before releasing the reset? > + // request irq > + irq = platform_get_irq(plat_dev, 0); > + if (irq < 0) { > + dev_err(&plat_dev->dev, "platform_get_irq failed\n"); > + irq = IRQ_NOTCONNECTED; By doing this you are making devm_request_irq() below return -ENOTCONN. Why not return the real error code right here? return dev_err_probe(&plat_dev->dev, irq, "platform_get_irq failed\n"); > + } > + > + ret = devm_request_irq(&plat_dev->dev, irq, rtc_irq_handler, > + IRQF_TRIGGER_RISING, "rtc irq", plat_dev); > + if (ret) { > + dev_err(&plat_dev->dev, "devm_request_irq failed: %d\n", ret); > + return ret; This could be shortened to: return dev_err_probe(&plat_dev->dev, ret, "devm_request_irq failed\n"); > + } > + > + // Setup trickle charger > + if (plat_dev->dev.of_node) > + sp_rtc_set_trickle_charger(plat_dev->dev); > + > + // reset > + sp_rtc.rstc = devm_reset_control_get_exclusive(&plat_dev->dev, NULL); > + if (IS_ERR(sp_rtc.rstc)) { > + ret = dev_err_probe(&plat_dev->dev, PTR_ERR(sp_rtc.rstc), > + "failed to retrieve reset controller\n"); > + return PTR_ERR(sp_rtc.rstc); This could be shortened to: return dev_err_probe(&plat_dev->dev, PTR_ERR(sp_rtc.rstc), "failed to retrieve reset controller\n"); > + } > + > + // clk > + sp_rtc.rtcclk = devm_clk_get(&plat_dev->dev, NULL); > + if (IS_ERR(sp_rtc.rtcclk)) { > + dev_err(&plat_dev->dev, "devm_clk_get fail\n"); > + return PTR_ERR(sp_rtc.rtcclk); > + } > + > + ret = reset_control_deassert(sp_rtc.rstc); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(sp_rtc.rtcclk); > + if (ret) Assert reset on failure? > + return ret; > + > + device_init_wakeup(&plat_dev->dev, 1); > + > + rtc = devm_rtc_allocate_device(&plat_dev->dev); > + if (IS_ERR(rtc)) Disable clock and assert reset on failure? > + return PTR_ERR(rtc); > + > + rtc->range_max = U32_MAX; > + rtc->range_min = 0; > + rtc->ops = rtc_ops; > + > + ret = devm_rtc_register_device(rtc); > + if (ret) > + goto free_reset_assert_clk; > + > + platform_set_drvdata(plat_dev, rtc); > + dev_info(&plat_dev->dev, "sp7021-rtc loaded\n"); > + > + return 0; > + > +free_reset_assert_clk: > + reset_control_assert(sp_rtc.rstc); > + clk_disable_unprepare(sp_rtc.rtcclk); Should this be the other way around? I'd expect this to be in the opposite order of reset deassert and clock enable. > + > + return ret; > +} > + > +static int sp_rtc_remove(struct platform_device *plat_dev) > +{ > + reset_control_assert(sp_rtc.rstc); > + clk_disable_unprepare(sp_rtc.rtcclk); Same as above. regards Philipp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 2021-11-09 6:38 ` [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 Vincent Shih 2021-11-29 9:45 ` Philipp Zabel @ 2021-11-29 11:20 ` Alexandre Belloni 1 sibling, 0 replies; 6+ messages in thread From: Alexandre Belloni @ 2021-11-29 11:20 UTC (permalink / raw) To: Vincent Shih Cc: a.zummo, p.zabel, linux-kernel, linux-rtc, robh+dt, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih, Vincent Shih Hello, On 09/11/2021 14:38:17+0800, Vincent Shih wrote: > +#define RTC_REG_NAME "rtc_reg" > + > +#define RTC_CTRL 0x40 > +#define TIMER_FREEZE (0x1 << 5) > +#define TIMER_FREEZE_MASK (1 << (5 + 16)) > +#define DIS_SYS_RST_RTC (0x1 << 4) > +#define DIS_SYS_RST_RTC_MASK (1 << (4 + 16)) > +#define RTC32K_MODE_RESET (0x1 << 3) > +#define RTC32K_MODE_RESET_MASK (1 << (3 + 16)) > +#define ALARM_EN_OVERDUE (0x1 << 2) > +#define ALARM_EN_OVERDUE_MASK (1 << (2 + 16)) > +#define ALARM_EN_PMC (0x1 << 1) > +#define ALARM_EN_PMC_MASK (1 << (1 + 16)) > +#define ALARM_EN (0x1 << 0) > +#define ALARM_EN_MASK (1 << (0 + 16)) You should definitively use BIT() and probably GENMASK > +#define RTC_TIMER_OUT 0x44 > +#define RTC_DIVIDER 0x48 > +#define RTC_TIMER_SET 0x4c > +#define RTC_ALARM_SET 0x50 > +#define RTC_USER_DATA 0x54 > +#define RTC_RESET_RECORD 0x58 > +#define RTC_BATTERY_CTRL 0x5c > +#define BAT_CHARGE_RSEL_2K_OHM (0x0 << 2) > +#define BAT_CHARGE_RSEL_250_OHM (0x1 << 2) > +#define BAT_CHARGE_RSEL_50_OHM (0x2 << 2) > +#define BAT_CHARGE_RSEL_0_OHM (0x3 << 2) Using FIELD_GET and FIELD_PREP later on will simplify those defines. > +#define BAT_CHARGE_RSEL_MASK (0x3 << (2 + 16)) > +#define BAT_CHARGE_DSEL_ON (0x0 << 1) > +#define BAT_CHARGE_DSEL_OFF (0x1 << 1) > +#define BAT_CHARGE_DSEL_MASK (0x1 << (1 + 16)) > +#define BAT_CHARGE_EN (0x1 << 0) > +#define BAT_CHARGE_EN_MASK (0x1 << (0 + 16)) > +#define RTC_TRIM_CTRL 0x60 > + > +struct sunplus_rtc { > + struct clk *rtcclk; > + struct reset_control *rstc; > + void __iomem *base; > + u32 ohms; > +}; > + > +struct sunplus_rtc sp_rtc; > + > +static void sp_get_seconds(unsigned long *secs) > +{ > + *secs = (unsigned long)readl(sp_rtc.base + RTC_TIMER_OUT); > +} > + > +static void sp_set_seconds(unsigned long secs) > +{ > + writel((u32)secs, sp_rtc.base + RTC_TIMER_SET); > +} > + > +static int sp_rtc_read_time(struct device *dev, struct rtc_time *tm) > +{ > + unsigned long secs; > + > + sp_get_seconds(&secs); > + rtc_time64_to_tm(secs, tm); > + dev_dbg(dev, "%s: RTC date/time to %d-%d-%d, %02d:%02d:%02d.\r\n", > + __func__, tm->tm_mday, tm->tm_mon + 1, tm->tm_year, > + tm->tm_hour, tm->tm_min, tm->tm_sec); You should use %ptR or simplify remove this debug message as the core has a tracepoint > + > + return rtc_valid_tm(tm); This is uneccessary as the first thing the core does is check the validity. > +} > + > +int sp_rtc_get_time(struct rtc_time *tm) This function seems unused. > +{ > + unsigned long secs; > + > + sp_get_seconds(&secs); > + rtc_time64_to_tm(secs, tm); > + > + return 0; > +} > + > +static int sp_rtc_suspend(struct platform_device *pdev, pm_message_t state) > +{ > + dev_dbg(&pdev->dev, "[RTC] Debug: %s(%d)\n", __func__, __LINE__); > + > + // Keep RTC from system reset > + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); > + > + return 0; > +} > + > +static int sp_rtc_resume(struct platform_device *pdev) > +{ > + /* */ > + /* Because RTC is still powered during suspend, */ > + /* there is nothing to do here. */ > + /* */ I'd prefer this comment to use a more common formatting. > + dev_dbg(&pdev->dev, "[RTC] Debug: %s(%d)\n", __func__, __LINE__); > + > + // Keep RTC from system reset > + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); > + Isn't that already done on suspend? > + return 0; > +} > + > +static int sp_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + unsigned long secs; > + > + secs = rtc_tm_to_time64(tm); > + dev_dbg(dev, "%s, secs = %lu\n", __func__, secs); > + sp_set_seconds(secs); > + > + return 0; > +} > + > +static int sp_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm) > +{ > + unsigned long alarm_time; > + > + alarm_time = rtc_tm_to_time64(&alrm->time); > + dev_dbg(dev, "%s, alarm_time: %u\n", __func__, (u32)(alarm_time)); > + writel((u32)alarm_time, sp_rtc.base + RTC_ALARM_SET); > + > + return 0; > +} > + > +static int sp_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm) > +{ > + unsigned int alarm_time; > + > + alarm_time = readl(sp_rtc.base + RTC_ALARM_SET); > + dev_dbg(dev, "%s, alarm_time: %u\n", __func__, alarm_time); > + > + if (alarm_time == 0) { > + // alarm is disabled This comment is not really useful > + alrm->enabled = 0; > + } else { > + // alarm is enabled Ditto > + alrm->enabled = 1; > + rtc_time64_to_tm((unsigned long)(alarm_time), &alrm->time); This should be done in either case. > + } > + > + return 0; > +} > + > +static int sp_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) > +{ > + if (enabled) > + writel((TIMER_FREEZE_MASK | DIS_SYS_RST_RTC_MASK | RTC32K_MODE_RESET_MASK | > + ALARM_EN_OVERDUE_MASK | ALARM_EN_PMC_MASK | ALARM_EN_MASK) | > + (DIS_SYS_RST_RTC | ALARM_EN_OVERDUE | ALARM_EN_PMC | > + ALARM_EN), sp_rtc.base + RTC_CTRL); > + else > + writel((ALARM_EN_OVERDUE_MASK | ALARM_EN_PMC_MASK | ALARM_EN_MASK) | 0x0, > + sp_rtc.base + RTC_CTRL); > + > + return 0; > +} > + > +static const struct rtc_class_ops sp_rtc_ops = { > + .read_time = sp_rtc_read_time, > + .set_time = sp_rtc_set_time, > + .set_alarm = sp_rtc_set_alarm, > + .read_alarm = sp_rtc_read_alarm, > + .alarm_irq_enable = sp_rtc_alarm_irq_enable, > +}; > + > +static irqreturn_t rtc_irq_handler(int irq, void *dev_id) > +{ > + struct platform_device *plat_dev = dev_id; > + struct rtc_device *rtc = platform_get_drvdata(plat_dev); > + > + rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF); > + dev_dbg(&plat_dev->dev, "[RTC] ALARM INT\n"); > + > + return IRQ_HANDLED; > +} > + > +/* ---------------------------------------------------------------------------------------------- */ > +/* bat_charge_rsel bat_charge_dsel bat_charge_en Remarks */ > +/* x x 0 Disable */ > +/* 0 0 1 0.86mA (2K Ohm with diode) */ > +/* 1 0 1 1.81mA (250 Ohm with diode) */ > +/* 2 0 1 2.07mA (50 Ohm with diode) */ > +/* 3 0 1 16.0mA (0 Ohm with diode) */ > +/* 0 1 1 1.36mA (2K Ohm without diode) */ > +/* 1 1 1 3.99mA (250 Ohm without diode) */ > +/* 2 1 1 4.41mA (50 Ohm without diode) */ > +/* 3 1 1 16.0mA (0 Ohm without diode) */ > +/* ---------------------------------------------------------------------------------------------- */ > +static void sp_rtc_set_trickle_charger(struct device dev) > +{ > + int ret; > + u32 rsel; > + > + ret = of_property_read_u32(dev.of_node, "trickle-resistor-ohms", &sp_rtc.ohms); > + if (ret) > + return; > + > + switch (sp_rtc.ohms) { > + case 2000: > + rsel = BAT_CHARGE_RSEL_2K_OHM; > + break; > + case 250: > + rsel = BAT_CHARGE_RSEL_250_OHM; > + break; > + case 50: > + rsel = BAT_CHARGE_RSEL_50_OHM; > + break; > + case 0: > + rsel = BAT_CHARGE_RSEL_0_OHM; > + break; > + default: > + return; > + } > + > + writel(BAT_CHARGE_RSEL_MASK | rsel, sp_rtc.base + RTC_BATTERY_CTRL); > + > + ret = of_property_read_bool(dev.of_node, "trickle-diode-disable"); This property is deprecated and used to be about disabling the charger, it has been replaced by aux-voltage-chargeable. I guess we need a new property, probably "trickle-diode" that would take a string parameter as some RTCs will need to know which type of diode has to be enabled (e.g. rv1805 would need "standard" or "schottky") > + if (ret) > + writel(BAT_CHARGE_DSEL_MASK | BAT_CHARGE_DSEL_OFF, sp_rtc.base + RTC_BATTERY_CTRL); > + else > + writel(BAT_CHARGE_DSEL_MASK | BAT_CHARGE_DSEL_ON, sp_rtc.base + RTC_BATTERY_CTRL); > + > + writel(BAT_CHARGE_EN_MASK | BAT_CHARGE_EN, sp_rtc.base + RTC_BATTERY_CTRL); > +} > + > +static int sp_rtc_probe(struct platform_device *plat_dev) > +{ > + struct rtc_device *rtc; > + struct resource *res; > + const struct rtc_class_ops *rtc_ops; > + void __iomem *reg_base; > + int ret, irq; > + > + memset(&sp_rtc, 0, sizeof(sp_rtc)); > + > + // find and map our resources > + res = platform_get_resource_byname(plat_dev, IORESOURCE_MEM, RTC_REG_NAME); > + > + if (res) { > + dev_dbg(&plat_dev->dev, "res = 0x%x\n", res->start); > + > + reg_base = devm_ioremap_resource(&plat_dev->dev, res); > + if (IS_ERR(reg_base)) { > + dev_err(&plat_dev->dev, "%s devm_ioremap_resource fail\n", RTC_REG_NAME); > + return PTR_ERR(reg_base); > + } > + > + dev_dbg(&plat_dev->dev, "reg_base = 0x%lx\n", (unsigned long)reg_base); > + } > + > + sp_rtc.base = reg_base; > + rtc_ops = &sp_rtc_ops; > + > + // Keep RTC from system reset > + writel(DIS_SYS_RST_RTC_MASK | DIS_SYS_RST_RTC, sp_rtc.base + RTC_CTRL); > + I guess this is already done in .suspend, are you sure you need that in all those locations? > + // request irq Useless comment > + irq = platform_get_irq(plat_dev, 0); > + if (irq < 0) { > + dev_err(&plat_dev->dev, "platform_get_irq failed\n"); > + irq = IRQ_NOTCONNECTED; > + } > + > + ret = devm_request_irq(&plat_dev->dev, irq, rtc_irq_handler, > + IRQF_TRIGGER_RISING, "rtc irq", plat_dev); > + if (ret) { > + dev_err(&plat_dev->dev, "devm_request_irq failed: %d\n", ret); > + return ret; > + } > + > + // Setup trickle charger > + if (plat_dev->dev.of_node) > + sp_rtc_set_trickle_charger(plat_dev->dev); > + > + // reset > + sp_rtc.rstc = devm_reset_control_get_exclusive(&plat_dev->dev, NULL); > + if (IS_ERR(sp_rtc.rstc)) { > + ret = dev_err_probe(&plat_dev->dev, PTR_ERR(sp_rtc.rstc), > + "failed to retrieve reset controller\n"); > + return PTR_ERR(sp_rtc.rstc); > + } > + > + // clk > + sp_rtc.rtcclk = devm_clk_get(&plat_dev->dev, NULL); > + if (IS_ERR(sp_rtc.rtcclk)) { > + dev_err(&plat_dev->dev, "devm_clk_get fail\n"); > + return PTR_ERR(sp_rtc.rtcclk); > + } > + > + ret = reset_control_deassert(sp_rtc.rstc); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(sp_rtc.rtcclk); > + if (ret) > + return ret; > + > + device_init_wakeup(&plat_dev->dev, 1); > + > + rtc = devm_rtc_allocate_device(&plat_dev->dev); > + if (IS_ERR(rtc)) > + return PTR_ERR(rtc); > + > + rtc->range_max = U32_MAX; > + rtc->range_min = 0; > + rtc->ops = rtc_ops; > + > + ret = devm_rtc_register_device(rtc); > + if (ret) > + goto free_reset_assert_clk; > + > + platform_set_drvdata(plat_dev, rtc); > + dev_info(&plat_dev->dev, "sp7021-rtc loaded\n"); This message needs to be removed, it is wrong and a proper message is already printed by the core. > + > + return 0; > + > +free_reset_assert_clk: > + reset_control_assert(sp_rtc.rstc); > + clk_disable_unprepare(sp_rtc.rtcclk); > + > + return ret; > +} > + > +static int sp_rtc_remove(struct platform_device *plat_dev) > +{ > + reset_control_assert(sp_rtc.rstc); > + clk_disable_unprepare(sp_rtc.rtcclk); > + > + return 0; > +} > + > +static const struct of_device_id sp_rtc_of_match[] = { > + { .compatible = "sunplus,sp7021-rtc" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, sp_rtc_of_match); > + > +static struct platform_driver sp_rtc_driver = { > + .probe = sp_rtc_probe, > + .remove = sp_rtc_remove, > + .suspend = sp_rtc_suspend, > + .resume = sp_rtc_resume, > + .driver = { > + .name = "sp7021-rtc", > + .owner = THIS_MODULE, > + .of_match_table = sp_rtc_of_match, > + }, > +}; > +module_platform_driver(sp_rtc_driver); > + > +MODULE_AUTHOR("Vincent Shih <vincent.shih@sunplus.com>"); > +MODULE_DESCRIPTION("Sunplus RTC driver"); > +MODULE_LICENSE("GPL v2"); > + > -- > 2.7.4 > -- Alexandre Belloni, co-owner and COO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema 2021-11-09 6:38 [PATCH v2 0/2] Add RTC driver for Sunplus SP7021 SoC Vincent Shih 2021-11-09 6:38 ` [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 Vincent Shih @ 2021-11-09 6:38 ` Vincent Shih 2021-11-29 1:31 ` Rob Herring 1 sibling, 1 reply; 6+ messages in thread From: Vincent Shih @ 2021-11-09 6:38 UTC (permalink / raw) To: a.zummo, alexandre.belloni, p.zabel, linux-kernel, linux-rtc, robh+dt, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih Cc: Vincent Shih Convert Sunplus RTC to json-schema Signed-off-by: Vincent Shih <vincent.shih@sunplus.com> --- Changes in v2: - Removed the header file of dt-bindings/clock/sp-sp7021.h - Removed the header file of dt-bindings/reset/sp-sp7021.h - Modified some statements after removing the header files .../bindings/rtc/sunplus,sp7021-rtc.yaml | 56 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml diff --git a/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml b/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml new file mode 100644 index 0000000..e74e015 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) Sunplus Co., Ltd. 2021 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/sunplus,sp7021-rtc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sunplus SP7021 Real Time Clock controller + +maintainers: + - Vincent Shih <vincent.shih@sunplus.com> + +properties: + compatible: + const: sunplus,sp7021-rtc + + reg: + maxItems: 1 + + reg-names: + items: + - const: rtc_reg + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - reg-names + - clocks + - resets + - interrupts + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + rtc: serial@9c003A00 { + compatible = "sunplus,sp7021-rtc"; + reg = <0x9c003A00 0x80>; + reg-names = "rtc_reg"; + clocks = <&clkc 0x12>; + resets = <&rstc 0x02>; + interrupt-parent = <&intc>; + interrupts = <163 IRQ_TYPE_EDGE_RISING>; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 6c1a535..c6774d1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17949,6 +17949,7 @@ SUNPLUS RTC DRIVER M: Vincent Shih <vincent.shih@sunplus.com> L: linux-rtc@vger.kernel.org S: Maintained +F: Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml F: drivers/rtc/rtc-sunplus.c SUPERH -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema 2021-11-09 6:38 ` [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema Vincent Shih @ 2021-11-29 1:31 ` Rob Herring 0 siblings, 0 replies; 6+ messages in thread From: Rob Herring @ 2021-11-29 1:31 UTC (permalink / raw) To: Vincent Shih Cc: a.zummo, alexandre.belloni, p.zabel, linux-kernel, linux-rtc, devicetree, wells.lu, in-reply-to=1635834123-24668-1-git-send-email-vincent.shih, Vincent Shih On Tue, Nov 09, 2021 at 02:38:18PM +0800, Vincent Shih wrote: > Convert Sunplus RTC to json-schema You are adding, not converting. > > Signed-off-by: Vincent Shih <vincent.shih@sunplus.com> The author (From) must match S-o-b. > --- > Changes in v2: > - Removed the header file of dt-bindings/clock/sp-sp7021.h > - Removed the header file of dt-bindings/reset/sp-sp7021.h > - Modified some statements after removing the header files > > .../bindings/rtc/sunplus,sp7021-rtc.yaml | 56 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml > > diff --git a/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml b/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml > new file mode 100644 > index 0000000..e74e015 > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml > @@ -0,0 +1,56 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) Sunplus Co., Ltd. 2021 > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rtc/sunplus,sp7021-rtc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sunplus SP7021 Real Time Clock controller > + > +maintainers: > + - Vincent Shih <vincent.shih@sunplus.com> > + > +properties: > + compatible: > + const: sunplus,sp7021-rtc > + > + reg: > + maxItems: 1 > + > + reg-names: > + items: > + - const: rtc_reg reg-names is kind of pointless here. At a minimum, '_reg' is redundant. > + > + clocks: > + maxItems: 1 > + > + resets: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - resets > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + rtc: serial@9c003A00 { rtc@9c003a00 > + compatible = "sunplus,sp7021-rtc"; > + reg = <0x9c003A00 0x80>; Use consistent case for hex (lower case). > + reg-names = "rtc_reg"; > + clocks = <&clkc 0x12>; > + resets = <&rstc 0x02>; > + interrupt-parent = <&intc>; > + interrupts = <163 IRQ_TYPE_EDGE_RISING>; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 6c1a535..c6774d1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17949,6 +17949,7 @@ SUNPLUS RTC DRIVER > M: Vincent Shih <vincent.shih@sunplus.com> > L: linux-rtc@vger.kernel.org > S: Maintained > +F: Documentation/devicetree/bindings/rtc/sunplus,sp7021-rtc.yaml > F: drivers/rtc/rtc-sunplus.c > > SUPERH > -- > 2.7.4 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-11-29 11:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-11-09 6:38 [PATCH v2 0/2] Add RTC driver for Sunplus SP7021 SoC Vincent Shih 2021-11-09 6:38 ` [PATCH v2 1/2] rtc: Add driver for Sunplus SP7021 Vincent Shih 2021-11-29 9:45 ` Philipp Zabel 2021-11-29 11:20 ` Alexandre Belloni 2021-11-09 6:38 ` [PATCH v2 2/2] dt-bindings: rtc: Convert Sunplus RTC to json-schema Vincent Shih 2021-11-29 1:31 ` Rob Herring
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).