* [linux-sunxi] [PATCH v4 2/2] media: V3s: Add support for Allwinner CSI.
From: Philippe Ombredanne @ 2017-12-22 13:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222102156.cfemen6ouxxxbrem@plaes.org>
Yong,
On Fri, Dec 22, 2017 at 11:21 AM, Priit Laes <plaes@plaes.org> wrote:
> On Fri, Dec 22, 2017 at 05:47:00PM +0800, Yong Deng wrote:
>> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
>> and CSI1 is used for parallel interface. This is not documented in
>> datasheet but by testing and guess.
>>
>> This patch implement a v4l2 framework driver for it.
>>
>> Currently, the driver only support the parallel interface. MIPI-CSI2,
>> ISP's support are not included in this patch.
>>
>> Signed-off-by: Yong Deng <yong.deng@magewell.com>
<snip>
>> --- /dev/null
>> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
>> @@ -0,0 +1,878 @@
>> +/*
>> + * Copyright (c) 2017 Magewell Electronics Co., Ltd. (Nanjing).
>> + * All rights reserved.
>> + * Author: Yong Deng <yong.deng@magewell.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
Would you mind using the new SPDX tags documented in Thomas patch set
[1] rather than this fine but longer legalese?
>> +MODULE_LICENSE("GPL v2");
Per module.h this means GPL2 only. This is not matching your top
license above which is GPL2 or later.
Please make sure your MODULE_LICENSE is consistent with the top level license.
[1] https://lkml.org/lkml/2017/12/4/934
--
Cordially
Philippe Ombredanne
^ permalink raw reply
* [PATCH v5 5/5] misc serdev: w2sg0004: add debugging code and Kconfig
From: Johan Hovold @ 2017-12-22 12:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5816bfb9e7cab68591c133e20696d6188ebe70de.1512114577.git.hns@goldelico.com>
On Fri, Dec 01, 2017 at 08:49:38AM +0100, H. Nikolaus Schaller wrote:
> This allows to set CONFIG_W2SG0004_DEBUG which will
> make the driver report more activities and it will turn on the
> GPS module during boot while the driver assumes that it
> is off. This can be used to debug the correct functioning of
> the hardware. Therefore we add it as an option to the driver
> because we think it is of general use (and a little tricky to
> add by system testers).
>
> Normally it should be off.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/misc/Kconfig | 8 ++++++++
> drivers/misc/w2sg0004.c | 37 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
I'd say say this does not belong in the kernel at all. And even if the
power-state test were to be allowed, most of the pr_debugs would
need to go. You really should be using dev_dbg and friends, which can
already be enabled selectively at run time using dynamic debugging.
Johan
^ permalink raw reply
* [RFC PATCH V1 1/2] clk: use atomic runtime pm api in clk_core_is_enabled
From: Ulf Hansson @ 2017-12-22 12:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513935965-12909-1-git-send-email-aisheng.dong@nxp.com>
On 22 December 2017 at 10:46, Dong Aisheng <aisheng.dong@nxp.com> wrote:
> Current clk_pm_runtime_put is using pm_runtime_put_sync which
> is not safe to be called in clk_core_is_enabled as it should
> be able to run in atomic context.
>
> Thus use pm_runtime_put instead which is atomic safe.
>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Fixes: 9a34b45397e5 ("clk: Add support for runtime PM")
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/clk/clk.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 5ec5809..e24968f 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -227,7 +227,8 @@ static bool clk_core_is_enabled(struct clk_core *core)
>
> ret = core->ops->is_enabled(core->hw);
> done:
> - clk_pm_runtime_put(core);
> + if (core->dev)
> + pm_runtime_put(core->dev);
>
> return ret;
> }
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v5 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: Johan Hovold @ 2017-12-22 12:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5494ad34b39a6c62601e3747440268dfb3be7d5a.1512114576.git.hns@goldelico.com>
On Fri, Dec 01, 2017 at 08:49:36AM +0100, H. Nikolaus Schaller wrote:
> Add driver for Wi2Wi W2SG0004/84 GPS module connected to some SoC UART.
>
> It uses serdev API hooks to monitor and forward the UART traffic to /dev/ttyGPSn
> and turn on/off the module. It also detects if the module is turned on (sends data)
> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>
> Additionally, rfkill block/unblock can be used to control an external LNA
> (and power down the module if not needed).
>
> The driver concept is based on code developed by Neil Brown <neilb@suse.de>
> but simplified and adapted to use the new serdev API introduced in v4.11.
I'm sorry (and I know this discussion has been going on for a long
time), but this still feels like too much of a hack.
You're registering a tty driver to allow user space to continue treat
this as a tty device, but you provide no means of actually modifying
anything (line settings, etc). It's essentially just a character device
with common tty ioctls as noops from a device PoV (well, plus the ldisc
buffering and processing).
This will probably require someone to first implement a generic gps
framework with a properly defined interface which you may then teach
gpsd to use (e.g. to avoid all its autodetection functionality) instead.
Or some entirely different approach, for example, where you manage
everything from user space.
I'd suggest reiterating the problem you're trying to solve and
enumerating the previously discussed potential solutions in order to
find a proper abstraction level for this (before getting lost in
implementation details).
That being said, I'm still pointing some bugs and issue below that you
can consider for future versions of this (and other drivers) below.
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/misc/Kconfig | 10 +
> drivers/misc/Makefile | 1 +
> drivers/misc/w2sg0004.c | 553 ++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 564 insertions(+)
> create mode 100644 drivers/misc/w2sg0004.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c2357b14..a3b11016ed2b 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -508,4 +508,14 @@ source "drivers/misc/mic/Kconfig"
> source "drivers/misc/genwqe/Kconfig"
> source "drivers/misc/echo/Kconfig"
> source "drivers/misc/cxl/Kconfig"
> +
> +config W2SG0004
> + tristate "W2SG00x4 on/off control"
Please provide a better summary for what this driver does.
> + depends on GPIOLIB && SERIAL_DEV_BUS
> + help
> + Enable on/off control of W2SG00x4 GPS moduled connected
Some whitespace issue here.
> + to some SoC UART to allow powering up/down if the /dev/ttyGPSn
> + is opened/closed.
> + It also provides a rfkill gps name to control the LNA power.
> +
> endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 5ca5f64df478..d9d824b3d20a 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
> obj-y += mic/
> obj-$(CONFIG_GENWQE) += genwqe/
> obj-$(CONFIG_ECHO) += echo/
> +obj-$(CONFIG_W2SG0004) += w2sg0004.o
> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
> obj-$(CONFIG_CXL_BASE) += cxl/
> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
> new file mode 100644
> index 000000000000..6bfd12eb8e02
> --- /dev/null
> +++ b/drivers/misc/w2sg0004.c
> @@ -0,0 +1,553 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
> + *
> + * Copyright (C) 2013 Neil Brown <neilb@suse.de>
> + * Copyright (C) 2015-2017 H. Nikolaus Schaller <hns@goldelico.com>,
> + * Golden Delicious Computers
> + *
> + * This receiver has an ON/OFF pin which must be toggled to
> + * turn the device 'on' of 'off'. A high->low->high toggle
s/of/or/
> + * will switch the device on if it is off, and off if it is on.
> + *
> + * To enable receiving on/off requests we register with the
> + * UART power management notifications.
No, the UART (serial device) would be the grandparent of your serdev
device (for which you register PM callbacks).
> + *
> + * It is not possible to directly detect the state of the device.
Didn't the 0084 version have a pin for this?
> + * However when it is on it will send characters on a UART line
> + * regularly.
> + *
> + * To detect that the power state is out of sync (e.g. if GPS
> + * was enabled before a reboot), we register for UART data received
> + * notifications.
> + *
> + * In addition we register as a rfkill client so that we can
> + * control the LNA power.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/rfkill.h>
> +#include <linux/serdev.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/workqueue.h>
> +
> +/*
> + * There seems to be restrictions on how quickly we can toggle the
> + * on/off line. data sheets says "two rtc ticks", whatever that means.
> + * If we do it too soon it doesn't work.
> + * So we have a state machine which uses the common work queue to ensure
> + * clean transitions.
> + * When a change is requested we record that request and only act on it
> + * once the previous change has completed.
> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
> + * one change per second.
> + */
> +
> +enum w2sg_state {
> + W2SG_IDLE, /* is not changing state */
> + W2SG_PULSE, /* activate on/off impulse */
> + W2SG_NOPULSE /* deactivate on/off impulse */
> +};
> +
> +struct w2sg_data {
> + struct rfkill *rf_kill;
> + struct regulator *lna_regulator;
> + int lna_blocked; /* rfkill block gps active */
> + int lna_is_off; /* LNA is currently off */
> + int is_on; /* current state (0/1) */
These look like flags; use a bitfield rather than an int.
> + unsigned long last_toggle;
> + unsigned long backoff; /* time to wait since last_toggle */
> + int on_off_gpio; /* the on-off gpio number */
> + struct serdev_device *uart; /* uart connected to the chip */
> + struct tty_driver *tty_drv; /* this is the user space tty */
This does not belong in the device data as someone (Arnd?) already
pointed out to you in a comment to an earlier version. More below.
> + struct device *dev; /* from tty_port_register_device() */
> + struct tty_port port;
> + int open_count; /* how often we were opened */
> + enum w2sg_state state;
> + int requested; /* requested state (0/1) */
> + int suspended;
> + struct delayed_work work;
> + int discard_count;
> +};
You seem to have completely ignored locking. These flags and resources
are accessed from multiple contexts, and it all looks very susceptible
to races (e.g. the work queue can race with the rfkill callback and
leave the regulator enabled when it should be off).
> +/* should become w2sg_by_minor[n] if we want to support multiple devices */
> +static struct w2sg_data *w2sg_by_minor;
> +
> +static int w2sg_set_lna_power(struct w2sg_data *data)
> +{
> + int ret = 0;
> + int off = data->suspended || !data->requested || data->lna_blocked;
> +
> + pr_debug("%s: %s\n", __func__, off ? "off" : "on");
This and other pr_xxx should be replaced with dev_dbg and friends.
> +
> + if (off != data->lna_is_off) {
> + data->lna_is_off = off;
> + if (!IS_ERR_OR_NULL(data->lna_regulator)) {
> + if (off)
> + regulator_disable(data->lna_regulator);
> + else
> + ret = regulator_enable(data->lna_regulator);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static void w2sg_set_power(void *pdata, int val)
Don't pass around void pointers like this.
> +{
> + struct w2sg_data *data = (struct w2sg_data *) pdata;
> +
> + pr_debug("%s to state=%d (requested=%d)\n", __func__, val,
> + data->requested);
> +
> + if (val && !data->requested) {
> + data->requested = true;
> + } else if (!val && data->requested) {
> + data->backoff = HZ;
> + data->requested = false;
> + } else
> + return;
Missing brackets.
> +
> + if (!data->suspended)
> + schedule_delayed_work(&data->work, 0);
> +}
> +
> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
> +static int w2sg_uart_receive_buf(struct serdev_device *serdev,
> + const unsigned char *rxdata,
> + size_t count)
> +{
> + struct w2sg_data *data =
> + (struct w2sg_data *) serdev_device_get_drvdata(serdev);
> +
> + if (!data->requested && !data->is_on) {
> + /*
> + * we have received characters while the w2sg
> + * should have been be turned off
> + */
> + data->discard_count += count;
> + if ((data->state == W2SG_IDLE) &&
> + time_after(jiffies,
> + data->last_toggle + data->backoff)) {
> + /* Should be off by now, time to toggle again */
> + pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
> + data->discard_count);
> +
> + data->discard_count = 0;
> +
> + data->is_on = true;
> + data->backoff *= 2;
> + if (!data->suspended)
> + schedule_delayed_work(&data->work, 0);
> + }
> + } else if (data->open_count > 0) {
> + int n;
> +
> + pr_debug("w2sg00x4: push %lu chars to tty port\n",
> + (unsigned long) count);
> +
> + /* pass to user-space */
> + n = tty_insert_flip_string(&data->port, rxdata, count);
> + if (n != count)
> + pr_err("w2sg00x4: did loose %lu characters\n",
> + (unsigned long) (count - n));
> + tty_flip_buffer_push(&data->port);
> + return n;
> + }
> +
> + /* assume we have processed everything */
> + return count;
> +}
> +
> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
> +static void toggle_work(struct work_struct *work)
> +{
> + struct w2sg_data *data = container_of(work, struct w2sg_data,
> + work.work);
> +
> + switch (data->state) {
> + case W2SG_IDLE:
> + if (data->requested == data->is_on)
> + return;
> +
> + w2sg_set_lna_power(data); /* update LNA power state */
> + gpio_set_value_cansleep(data->on_off_gpio, 0);
> + data->state = W2SG_PULSE;
> +
> + pr_debug("w2sg: power gpio ON\n");
> +
> + schedule_delayed_work(&data->work,
> + msecs_to_jiffies(10));
> + break;
> +
> + case W2SG_PULSE:
> + gpio_set_value_cansleep(data->on_off_gpio, 1);
> + data->last_toggle = jiffies;
> + data->state = W2SG_NOPULSE;
> + data->is_on = !data->is_on;
> +
> + pr_debug("w2sg: power gpio OFF\n");
> +
> + schedule_delayed_work(&data->work,
> + msecs_to_jiffies(10));
> + break;
> +
> + case W2SG_NOPULSE:
> + data->state = W2SG_IDLE;
> +
> + pr_debug("w2sg: idle\n");
> +
> + break;
> +
> + }
> +}
> +
> +static int w2sg_rfkill_set_block(void *pdata, bool blocked)
> +{
> + struct w2sg_data *data = pdata;
> +
> + pr_debug("%s: blocked: %d\n", __func__, blocked);
> +
> + data->lna_blocked = blocked;
> +
> + return w2sg_set_lna_power(data);
> +}
> +
> +static struct rfkill_ops w2sg0004_rfkill_ops = {
> + .set_block = w2sg_rfkill_set_block,
> +};
> +
> +static struct serdev_device_ops serdev_ops = {
> + .receive_buf = w2sg_uart_receive_buf,
> +};
> +
> +/*
> + * we are a man-in the middle between the user-space visible tty port
> + * and the serdev tty where the chip is connected.
> + * This allows us to recognise when the device should be powered on
> + * or off and handle the "false" state that data arrives while no
> + * users-space tty client exists.
> + */
> +
> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
> +{
> + BUG_ON(minor >= 1);
Never use BUG_ON in driver code.
> + return w2sg_by_minor;
> +}
> +
> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> + struct w2sg_data *data;
> + int retval;
> +
> + pr_debug("%s() tty = %p\n", __func__, tty);
> +
> + data = w2sg_get_by_minor(tty->index);
> +
> + if (!data)
> + return -ENODEV;
> +
> + retval = tty_standard_install(driver, tty);
> + if (retval)
> + goto error_init_termios;
> +
> + tty->driver_data = data;
> +
> + return 0;
> +
> +error_init_termios:
> + tty_port_put(&data->port);
Where's the corresponding get?
> + return retval;
> +}
> +
> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
> +{
> + struct w2sg_data *data = tty->driver_data;
> +
> + pr_debug("%s() data = %p open_count = ++%d\n", __func__,
> + data, data->open_count);
> +
> + w2sg_set_power(data, ++data->open_count > 0);
> +
> + return tty_port_open(&data->port, tty, file);
You'd leave the open count incremented on errors here.
> +}
> +
> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
> +{
> + struct w2sg_data *data = tty->driver_data;
> +
> + pr_debug("%s()\n", __func__);
> +
> + w2sg_set_power(data, --data->open_count > 0);
> +
> + tty_port_close(&data->port, tty, file);
> +}
> +
> +static int w2sg_tty_write(struct tty_struct *tty,
> + const unsigned char *buffer, int count)
> +{
> + struct w2sg_data *data = tty->driver_data;
> +
> + /* simply pass down to UART */
> + return serdev_device_write_buf(data->uart, buffer, count);
> +}
> +
> +static const struct tty_operations w2sg_serial_ops = {
> + .install = w2sg_tty_install,
> + .open = w2sg_tty_open,
> + .close = w2sg_tty_close,
> + .write = w2sg_tty_write,
> +};
> +
> +static const struct tty_port_operations w2sg_port_ops = {
> + /* none defined, but we need the struct */
> +};
> +
> +static int w2sg_probe(struct serdev_device *serdev)
> +{
> + struct w2sg_data *data;
> + struct rfkill *rf_kill;
> + int err;
> + int minor;
> + enum of_gpio_flags flags;
> +
> + pr_debug("%s()\n", __func__);
> +
> + /*
> + * For future consideration:
> + * for multiple such GPS receivers in one system
> + * we need a mechanism to define distinct minor values
> + * and search for an unused one.
> + */
> + minor = 0;
> + if (w2sg_get_by_minor(minor)) {
> + pr_err("w2sg minor is already in use!\n");
> + return -ENODEV;
> + }
> +
> + data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + serdev_device_set_drvdata(serdev, data);
> +
> + data->on_off_gpio = of_get_named_gpio_flags(serdev->dev.of_node,
> + "enable-gpios", 0,
> + &flags);
You should be using gpio descriptors and not the legacy interface.
> + /* defer until we have all gpios */
> + if (data->on_off_gpio == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + data->lna_regulator = devm_regulator_get_optional(&serdev->dev,
> + "lna");
> + if (IS_ERR(data->lna_regulator)) {
> + /* defer until we can get the regulator */
> + if (PTR_ERR(data->lna_regulator) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + data->lna_regulator = NULL;
> + }
> + pr_debug("%s() lna_regulator = %p\n", __func__, data->lna_regulator);
> +
> + data->lna_blocked = true;
> + data->lna_is_off = true;
> +
> + data->is_on = false;
> + data->requested = false;
> + data->state = W2SG_IDLE;
> + data->last_toggle = jiffies;
> + data->backoff = HZ;
> +
> + data->uart = serdev;
> +
> + INIT_DELAYED_WORK(&data->work, toggle_work);
> +
> + err = devm_gpio_request(&serdev->dev, data->on_off_gpio,
> + "w2sg0004-on-off");
> + if (err < 0)
> + goto out;
Just return unless you're actually undwinding something.
> +
> + gpio_direction_output(data->on_off_gpio, false);
> +
> + serdev_device_set_client_ops(data->uart, &serdev_ops);
> + serdev_device_open(data->uart);
Missing error handling.
And always keeping the port open would in most cases prevent the serial
controller from runtime suspending.
> +
> + serdev_device_set_baudrate(data->uart, 9600);
> + serdev_device_set_flow_control(data->uart, false);
So you hardcode these settings and provide no means for user space to
change them. This may make sense for this GPS, but it looks like at
least some of the wi2wi 0084 modules use 4800 baud ("i"-versions?).
> +
> + rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
> + &w2sg0004_rfkill_ops, data);
> + if (rf_kill == NULL) {
> + err = -ENOMEM;
> + goto err_rfkill;
Name error labels after what they do (not where you jump from).
That may have prevented the NULL deref you'd trigger in the error path
here.
> + }
> +
> + err = rfkill_register(rf_kill);
> + if (err) {
> + dev_err(&serdev->dev, "Cannot register rfkill device\n");
> + goto err_rfkill;
> + }
> +
> + data->rf_kill = rf_kill;
> +
> + /* allocate the tty driver */
> + data->tty_drv = alloc_tty_driver(1);
As was already pointed out by Arnd in a review of an previous version of
this series, this must not be done at probe (do it at module init). We
don't want separate tty drivers for every device.
> + if (!data->tty_drv)
> + return -ENOMEM;
Here you'd leak the registered rfkill structure, and leave the port
open.
> +
> + /* initialize the tty driver */
> + data->tty_drv->owner = THIS_MODULE;
> + data->tty_drv->driver_name = "w2sg0004";
> + data->tty_drv->name = "ttyGPS";
I don't think you should be claiming this generic namespace for
something as specific as this.
> + data->tty_drv->major = 0;
> + data->tty_drv->minor_start = 0;
> + data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
> + data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
> + data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
> + data->tty_drv->init_termios = tty_std_termios;
> + data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
> + HUPCL | CLOCAL;
> + /*
> + * optional:
> + * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
> + 115200, 115200);
> + * w2sg_tty_termios(&data->tty_drv->init_termios);
> + */
Why is this here?
> + tty_set_operations(data->tty_drv, &w2sg_serial_ops);
> +
> + /* register the tty driver */
> + err = tty_register_driver(data->tty_drv);
> + if (err) {
> + pr_err("%s - tty_register_driver failed(%d)\n",
> + __func__, err);
> + put_tty_driver(data->tty_drv);
> + goto err_rfkill;
> + }
> +
> + /* minor (0) is now in use */
> + w2sg_by_minor = data;
> +
> + tty_port_init(&data->port);
> + data->port.ops = &w2sg_port_ops;
> +
> + data->dev = tty_port_register_device(&data->port,
> + data->tty_drv, minor, &serdev->dev);
Missing error handling.
> +
> + /* keep off until user space requests the device */
> + w2sg_set_power(data, false);
> +
> + return 0;
> +
> +err_rfkill:
> + rfkill_destroy(rf_kill);
> + serdev_device_close(data->uart);
> +out:
> + return err;
> +}
> +
> +static void w2sg_remove(struct serdev_device *serdev)
> +{
> + struct w2sg_data *data = serdev_device_get_drvdata(serdev);
> + int minor;
> +
> + cancel_delayed_work_sync(&data->work);
> +
> + /* what is the right sequence to avoid problems? */
Clearly that's something you need to determine.
> + serdev_device_close(data->uart);
> +
> + /* we should lookup in w2sg_by_minor */
> + minor = 0;
> + tty_unregister_device(data->tty_drv, minor);
> +
> + tty_unregister_driver(data->tty_drv);
> +}
> +
> +static int __maybe_unused w2sg_suspend(struct device *dev)
> +{
> + struct w2sg_data *data = dev_get_drvdata(dev);
> +
> + data->suspended = true;
> +
> + cancel_delayed_work_sync(&data->work);
> +
> + w2sg_set_lna_power(data); /* shuts down if needed */
> +
> + if (data->state == W2SG_PULSE) {
> + msleep(10);
> + gpio_set_value_cansleep(data->on_off_gpio, 1);
> + data->last_toggle = jiffies;
> + data->is_on = !data->is_on;
> + data->state = W2SG_NOPULSE;
> + }
> +
> + if (data->state == W2SG_NOPULSE) {
> + msleep(10);
> + data->state = W2SG_IDLE;
> + }
> +
> + if (data->is_on) {
> + pr_info("GPS off for suspend %d %d %d\n", data->requested,
> + data->is_on, data->lna_is_off);
> +
> + gpio_set_value_cansleep(data->on_off_gpio, 0);
> + msleep(10);
> + gpio_set_value_cansleep(data->on_off_gpio, 1);
> + data->is_on = 0;
> + }
> +
> + return 0;
> +}
> +
> +static int __maybe_unused w2sg_resume(struct device *dev)
> +{
> + struct w2sg_data *data = dev_get_drvdata(dev);
> +
> + data->suspended = false;
> +
> + pr_info("GPS resuming %d %d %d\n", data->requested,
> + data->is_on, data->lna_is_off);
> +
> + schedule_delayed_work(&data->work, 0); /* enables LNA if needed */
> +
> + return 0;
> +}
> +
> +static const struct of_device_id w2sg0004_of_match[] = {
> + { .compatible = "wi2wi,w2sg0004" },
> + { .compatible = "wi2wi,w2sg0084" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
> +
> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_suspend, w2sg_resume);
> +
> +static struct serdev_device_driver w2sg_driver = {
> + .probe = w2sg_probe,
> + .remove = w2sg_remove,
> + .driver = {
> + .name = "w2sg0004",
> + .owner = THIS_MODULE,
> + .pm = &w2sg_pm_ops,
> + .of_match_table = of_match_ptr(w2sg0004_of_match)
> + },
> +};
> +
> +module_serdev_device_driver(w2sg_driver);
> +
> +MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
> +MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
> +MODULE_LICENSE("GPL v2");
Johan
^ permalink raw reply
* [PATCH] ARM: make ARCH_S3C24XX select USE_OF and clean-up boot/dts/Makefile
From: Krzysztof Kozlowski @ 2017-12-22 12:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1511749163-11057-1-git-send-email-yamada.masahiro@socionext.com>
On Mon, Nov 27, 2017 at 3:19 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> ARCH_S3C24XX is a very exceptional platform that some DT files in
> arch/arm/boot/dts/, but does not select USE_OF.
Not entirely. The platform does select USE_OF - when MACH_S3C2416_DT
is chosen. For other boards USE_OF is not necessary because they do
not use DT. Why you need to select it for entire arch?
Best regards,
Krzysztof
>
> All the other platforms with DT files correctly select USE_OF
> directly or indirectly (Most of them are either ARCH_MULTIPLATFORM
> or ARM_SINGLE_ARMV7M).
>
> With ARCH_S3C24XX fixed, "ifeq ($(CONFIG_OF),y)" in DT Makefile
> can be deleted.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> arch/arm/Kconfig | 1 +
> arch/arm/boot/dts/Makefile | 3 ---
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 51c8df5..5604497 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -595,6 +595,7 @@ config ARCH_S3C24XX
> select MULTI_IRQ_HANDLER
> select NEED_MACH_IO_H
> select SAMSUNG_ATAGS
> + select USE_OF
> help
> Samsung S3C2410, S3C2412, S3C2413, S3C2416, S3C2440, S3C2442, S3C2443
> and S3C2450 SoCs based systems, such as the Simtec Electronics BAST
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index d0381e9..6f7f25d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -1,6 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0
> -ifeq ($(CONFIG_OF),y)
> -
> dtb-$(CONFIG_ARCH_ALPINE) += \
> alpine-db.dtb
> dtb-$(CONFIG_MACH_ARTPEC6) += \
> @@ -1104,4 +1102,3 @@ dtb-$(CONFIG_ARCH_ZX) += zx296702-ad1.dtb
> dtb-$(CONFIG_ARCH_ASPEED) += aspeed-bmc-opp-palmetto.dtb \
> aspeed-bmc-opp-romulus.dtb \
> aspeed-ast2500-evb.dtb
> -endif
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 11/11] arm64: allwinner: a64: add simplefb for A64 SoC
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The A64 SoC features two display pipelines, one has a LCD output, the
other has a HDMI output.
Add support for simplefb for these pipelines on A64 SoC.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 31 +++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index fb8ea7c414e1..993f5df73e8d 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -42,9 +42,11 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#include <dt-bindings/clock/sun8i-de2.h>
#include <dt-bindings/clock/sun50i-a64-ccu.h>
#include <dt-bindings/clock/sun8i-r-ccu.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/sun8i-de2.h>
#include <dt-bindings/reset/sun50i-a64-ccu.h>
/ {
@@ -52,6 +54,35 @@
#address-cells = <1>;
#size-cells = <1>;
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer-lcd {
+ compatible = "allwinner,simple-framebuffer",
+ "simple-framebuffer";
+ allwinner,pipeline = "mixer0-lcd0";
+ clocks = <&display_clocks CLK_BUS_MIXER0>,
+ <&ccu CLK_BUS_TCON0>, <&ccu CLK_BUS_TCON0>,
+ <&display_clocks CLK_MIXER0>,
+ <&ccu CLK_TCON0>;
+ status = "disabled";
+ };
+
+ framebuffer-hdmi {
+ compatible = "allwinner,simple-framebuffer",
+ "simple-framebuffer";
+ allwinner,pipeline = "mixer1-lcd1-hdmi";
+ clocks = <&display_clocks CLK_BUS_MIXER1>,
+ <&ccu CLK_BUS_TCON1>, <&ccu CLK_BUS_HDMI>,
+ <&display_clocks CLK_MIXER1>,
+ <&ccu CLK_TCON1>, <&ccu CLK_HDMI>,
+ <&ccu CLK_HDMI_DDC>;
+ status = "disabled";
+ };
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.14.2
^ permalink raw reply related
* [PATCH v3 10/11] arm64: allwinner: a64: add DE2 CCU for A64 SoC
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The A64 SoC features a DE2 CCU like the one in H5, but needs to claim a
section of SRAM (SRAM C) to be accessed.
Adds the device tree nodes for the SRAM controller and the DE2 CCU.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v3:
- Fixed the alliwnner,sram property (the 1 after SRAM phadle is missing
in v2).
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 34 +++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index d783d164b9c3..fb8ea7c414e1 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -130,6 +130,40 @@
#size-cells = <1>;
ranges;
+ display_clocks: clock at 1000000 {
+ compatible = "allwinner,sun50i-a64-de2-clk";
+ reg = <0x01000000 0x100000>;
+ clocks = <&ccu CLK_DE>,
+ <&ccu CLK_BUS_DE>;
+ clock-names = "mod",
+ "bus";
+ resets = <&ccu RST_BUS_DE>;
+ allwinner,sram = <&de2_sram 1>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
+ sram-controller at 1c00000 {
+ compatible = "allwinner,sun50i-a64-sram-controller";
+ reg = <0x01c00000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ sram_c: sram at 18000 {
+ compatible = "mmio-sram";
+ reg = <0x00018000 0x28000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x00018000 0x28000>;
+
+ de2_sram: sram-section at 0 {
+ compatible = "allwinner,sun50i-a64-sram-c";
+ reg = <0x0000 0x28000>;
+ };
+ };
+ };
+
syscon: syscon at 1c00000 {
compatible = "allwinner,sun50i-a64-system-controller",
"syscon";
--
2.14.2
^ permalink raw reply related
* [PATCH v3 09/11] clk: sunxi-ng: add support for Allwinner A64 DE2 CCU
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
Allwinner A64's DE2 needs to claim a section of SRAM (SRAM C) to work.
Add support for it.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
drivers/clk/sunxi-ng/ccu-sun8i-de2.c | 32 ++++++++++++++++++++++++--------
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
index 468d1abaf0ee..38b029b7bb5a 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
@@ -17,6 +17,7 @@
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/reset.h>
+#include <linux/soc/sunxi/sunxi_sram.h>
#include "ccu_common.h"
#include "ccu_div.h"
@@ -196,6 +197,11 @@ static const struct sunxi_ccu_desc sun8i_v3s_de2_clk_desc = {
.num_resets = ARRAY_SIZE(sun8i_a83t_de2_resets),
};
+static bool sunxi_de2_clk_has_sram(const struct device_node *node)
+{
+ return of_device_is_compatible(node, "allwinner,sun50i-a64-de2-clk");
+}
+
static int sunxi_de2_clk_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -239,11 +245,20 @@ static int sunxi_de2_clk_probe(struct platform_device *pdev)
return ret;
}
+ if (sunxi_de2_clk_has_sram(pdev->dev.of_node)) {
+ ret = sunxi_sram_claim(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev,
+ "Error couldn't map SRAM to device\n");
+ return ret;
+ }
+ }
+
/* The clocks need to be enabled for us to access the registers */
ret = clk_prepare_enable(bus_clk);
if (ret) {
dev_err(&pdev->dev, "Couldn't enable bus clk: %d\n", ret);
- return ret;
+ goto err_release_sram;
}
ret = clk_prepare_enable(mod_clk);
@@ -272,6 +287,10 @@ static int sunxi_de2_clk_probe(struct platform_device *pdev)
clk_disable_unprepare(mod_clk);
err_disable_bus_clk:
clk_disable_unprepare(bus_clk);
+err_release_sram:
+ if (sunxi_de2_clk_has_sram(pdev->dev.of_node))
+ sunxi_sram_release(&pdev->dev);
+
return ret;
}
@@ -288,17 +307,14 @@ static const struct of_device_id sunxi_de2_clk_ids[] = {
.compatible = "allwinner,sun8i-v3s-de2-clk",
.data = &sun8i_v3s_de2_clk_desc,
},
+ {
+ .compatible = "allwinner,sun50i-a64-de2-clk",
+ .data = &sun50i_a64_de2_clk_desc,
+ },
{
.compatible = "allwinner,sun50i-h5-de2-clk",
.data = &sun50i_a64_de2_clk_desc,
},
- /*
- * The Allwinner A64 SoC needs some bit to be poke in syscon to make
- * DE2 really working.
- * So there's currently no A64 compatible here.
- * H5 shares the same reset line with A64, so here H5 is using the
- * clock description of A64.
- */
{ }
};
--
2.14.2
^ permalink raw reply related
* [PATCH v3 08/11] dt-bindings: add binding for A64 DE2 CCU SRAM
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
A64's Display Engine 2.0 needs a section of SRAM (SRAM C) to be claimed,
otherwise the whole DE2 memory zone cannot be accessed (kept to all 0).
Add binding for this, in order to make the DE2 CCU able to claim the
SRAM and enable access to the DE2 clock and reset registers.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v3:
- Add Rob's ACK.
Documentation/devicetree/bindings/clock/sun8i-de2.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/sun8i-de2.txt b/Documentation/devicetree/bindings/clock/sun8i-de2.txt
index f2fa87c4765c..a7d558a2b9b2 100644
--- a/Documentation/devicetree/bindings/clock/sun8i-de2.txt
+++ b/Documentation/devicetree/bindings/clock/sun8i-de2.txt
@@ -6,6 +6,7 @@ Required properties :
- "allwinner,sun8i-a83t-de2-clk"
- "allwinner,sun8i-h3-de2-clk"
- "allwinner,sun8i-v3s-de2-clk"
+ - "allwinner,sun50i-a64-de2-clk"
- "allwinner,sun50i-h5-de2-clk"
- reg: Must contain the registers base address and length
@@ -18,6 +19,10 @@ Required properties :
- #clock-cells : must contain 1
- #reset-cells : must contain 1
+Additional required properties for "allwinner,sun50i-a64-de2-clk" :
+- allwinner,sram: See Documentation/devicetree/bindings/sram/sunxi-sram.txt,
+ should be the SRAM C section on A64 SoC.
+
Example:
de2_clocks: clock at 1000000 {
compatible = "allwinner,sun8i-h3-de2-clk";
--
2.14.2
^ permalink raw reply related
* [PATCH v3 07/11] ARM: sunxi: h3/h5: add simplefb nodes
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The H3/H5 SoCs have a HDMI output and a TV Composite output.
Add simplefb nodes for these outputs.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index fcb909658cf0..31478c03790d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -53,6 +53,35 @@
#address-cells = <1>;
#size-cells = <1>;
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer-hdmi {
+ compatible = "allwinner,simple-framebuffer",
+ "simple-framebuffer";
+ allwinner,pipeline = "mixer0-lcd0-hdmi";
+ clocks = <&display_clocks CLK_BUS_MIXER0>,
+ <&ccu CLK_BUS_TCON0>, <&ccu CLK_BUS_HDMI>,
+ <&display_clocks CLK_MIXER0>,
+ <&ccu CLK_TCON0>, <&ccu CLK_HDMI>,
+ <&ccu CLK_HDMI_DDC>;
+ status = "disabled";
+ };
+
+ framebuffer-tve {
+ compatible = "allwinner,simple-framebuffer",
+ "simple-framebuffer";
+ allwinner,pipeline = "mixer1-lcd1-tve";
+ clocks = <&display_clocks CLK_BUS_MIXER1>,
+ <&ccu CLK_BUS_TCON1>, <&ccu CLK_BUS_TVE>,
+ <&display_clocks CLK_MIXER1>,
+ <&ccu CLK_TVE>;
+ status = "disabled";
+ };
+ };
+
clocks {
#address-cells = <1>;
#size-cells = <1>;
--
2.14.2
^ permalink raw reply related
* [PATCH v3 06/11] arm64: allwinner: h5: add compatible string for DE2 CCU
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The DE2 CCU on Allwinner H5 SoC has a slightly different behavior than
the one on H3, so the compatible string is not set in the common DTSI
file.
Add the compatible string of H5 DE2 CCU in H5 DTSI file.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
index d9a720bff05d..e237c05cfdb4 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi
@@ -98,6 +98,10 @@
compatible = "allwinner,sun50i-h5-ccu";
};
+&display_clocks {
+ compatible = "allwinner,sun50i-h5-de2-clk";
+};
+
&mmc0 {
compatible = "allwinner,sun50i-h5-mmc",
"allwinner,sun50i-a64-mmc";
--
2.14.2
^ permalink raw reply related
* [PATCH v3 05/11] ARM: sun8i: h3/h5: add DE2 CCU device node for H3
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The DE2 in H3/H5 has a clock control unit in it, and the behavior is
slightly different between H3 and H5.
Add the common parts in H3/H5 DTSI, and add the compatible string in H3
DTSI.
The compatible string of H5 DE2 CCU will be added in a separated patch.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Use H3 DE2 CCU compatible as it's discovered that H3 and A83T DE2 CCU are
not equal.
arch/arm/boot/dts/sun8i-h3.dtsi | 4 ++++
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 14 ++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
index b36f9f423c39..8495deecedad 100644
--- a/arch/arm/boot/dts/sun8i-h3.dtsi
+++ b/arch/arm/boot/dts/sun8i-h3.dtsi
@@ -85,6 +85,10 @@
compatible = "allwinner,sun8i-h3-ccu";
};
+&display_clocks {
+ compatible = "allwinner,sun8i-h3-de2-clk";
+};
+
&mmc0 {
compatible = "allwinner,sun7i-a20-mmc";
clocks = <&ccu CLK_BUS_MMC0>,
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 8d40c00d64bb..fcb909658cf0 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -40,9 +40,11 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#include <dt-bindings/clock/sun8i-de2.h>
#include <dt-bindings/clock/sun8i-h3-ccu.h>
#include <dt-bindings/clock/sun8i-r-ccu.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/sun8i-de2.h>
#include <dt-bindings/reset/sun8i-h3-ccu.h>
#include <dt-bindings/reset/sun8i-r-ccu.h>
@@ -85,6 +87,18 @@
#size-cells = <1>;
ranges;
+ display_clocks: clock at 1000000 {
+ /* compatible is in per SoC .dtsi file */
+ reg = <0x01000000 0x100000>;
+ clocks = <&ccu CLK_DE>,
+ <&ccu CLK_BUS_DE>;
+ clock-names = "mod",
+ "bus";
+ resets = <&ccu RST_BUS_DE>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ };
+
syscon: syscon at 1c00000 {
compatible = "allwinner,sun8i-h3-system-controller",
"syscon";
--
2.14.2
^ permalink raw reply related
* [PATCH v3 04/11] dt-bindings: simplefb-sunxi: add pipelines for DE2
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
As we're going to add simplefb support for Allwinner SoCs with DE2, add
suitable pipeline strings in the device tree binding.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Changes in v2:
- Adds Rob's ACK.
.../devicetree/bindings/display/simple-framebuffer-sunxi.txt | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt b/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
index a9168ae6946c..d693b8dc9a62 100644
--- a/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
+++ b/Documentation/devicetree/bindings/display/simple-framebuffer-sunxi.txt
@@ -15,6 +15,10 @@ Required properties:
"de_be1-lcd1"
"de_be0-lcd0-hdmi"
"de_be1-lcd1-hdmi"
+ "mixer0-lcd0"
+ "mixer0-lcd0-hdmi"
+ "mixer1-lcd1-hdmi"
+ "mixer1-lcd1-tve"
Example:
--
2.14.2
^ permalink raw reply related
* [PATCH v3 03/11] clk: sunxi-ng: fix the A64/H5 clock description of DE2 CCU
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The clocks of A64/H5 SoCs in the DE2 CCU is the same as the clocks in H3
DE2 CCU rather than the A83T DE2 CCU (the parent of them is the DE
module clock).
Fix this by change the clock descriptions to use the clocks of H3.
Fixes: 763c5bd045b1 ("clk: sunxi-ng: add support for DE2 CCU")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
drivers/clk/sunxi-ng/ccu-sun8i-de2.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
index 2db5d4e00ea7..468d1abaf0ee 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
@@ -177,10 +177,10 @@ static const struct sunxi_ccu_desc sun8i_h3_de2_clk_desc = {
};
static const struct sunxi_ccu_desc sun50i_a64_de2_clk_desc = {
- .ccu_clks = sun8i_a83t_de2_clks,
- .num_ccu_clks = ARRAY_SIZE(sun8i_a83t_de2_clks),
+ .ccu_clks = sun8i_h3_de2_clks,
+ .num_ccu_clks = ARRAY_SIZE(sun8i_h3_de2_clks),
- .hw_clks = &sun8i_a83t_de2_hw_clks,
+ .hw_clks = &sun8i_h3_de2_hw_clks,
.resets = sun50i_a64_de2_resets,
.num_resets = ARRAY_SIZE(sun50i_a64_de2_resets),
--
2.14.2
^ permalink raw reply related
* [PATCH v3 02/11] clk: sunxi-ng: add support for Allwinner H3 DE2 CCU
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
Allwinner H3 features a DE2 CCU like the one on A83T, however the
parent of the clocks is the DE module clock, not the PLL_DE clock.
Add support for it.
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
drivers/clk/sunxi-ng/ccu-sun8i-de2.c | 47 ++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
index 5cc9d9952121..2db5d4e00ea7 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-de2.c
@@ -41,6 +41,8 @@ static SUNXI_CCU_GATE(wb_clk, "wb", "wb-div",
static SUNXI_CCU_M(mixer0_div_clk, "mixer0-div", "de", 0x0c, 0, 4,
CLK_SET_RATE_PARENT);
+static SUNXI_CCU_M(mixer1_div_clk, "mixer1-div", "de", 0x0c, 4, 4,
+ CLK_SET_RATE_PARENT);
static SUNXI_CCU_M(wb_div_clk, "wb-div", "de", 0x0c, 8, 4,
CLK_SET_RATE_PARENT);
@@ -65,6 +67,20 @@ static struct ccu_common *sun8i_a83t_de2_clks[] = {
&wb_div_a83_clk.common,
};
+static struct ccu_common *sun8i_h3_de2_clks[] = {
+ &mixer0_clk.common,
+ &mixer1_clk.common,
+ &wb_clk.common,
+
+ &bus_mixer0_clk.common,
+ &bus_mixer1_clk.common,
+ &bus_wb_clk.common,
+
+ &mixer0_div_clk.common,
+ &mixer1_div_clk.common,
+ &wb_div_clk.common,
+};
+
static struct ccu_common *sun8i_v3s_de2_clks[] = {
&mixer0_clk.common,
&wb_clk.common,
@@ -93,6 +109,23 @@ static struct clk_hw_onecell_data sun8i_a83t_de2_hw_clks = {
.num = CLK_NUMBER,
};
+static struct clk_hw_onecell_data sun8i_h3_de2_hw_clks = {
+ .hws = {
+ [CLK_MIXER0] = &mixer0_clk.common.hw,
+ [CLK_MIXER1] = &mixer1_clk.common.hw,
+ [CLK_WB] = &wb_clk.common.hw,
+
+ [CLK_BUS_MIXER0] = &bus_mixer0_clk.common.hw,
+ [CLK_BUS_MIXER1] = &bus_mixer1_clk.common.hw,
+ [CLK_BUS_WB] = &bus_wb_clk.common.hw,
+
+ [CLK_MIXER0_DIV] = &mixer0_div_clk.common.hw,
+ [CLK_MIXER1_DIV] = &mixer1_div_clk.common.hw,
+ [CLK_WB_DIV] = &wb_div_clk.common.hw,
+ },
+ .num = CLK_NUMBER,
+};
+
static struct clk_hw_onecell_data sun8i_v3s_de2_hw_clks = {
.hws = {
[CLK_MIXER0] = &mixer0_clk.common.hw,
@@ -133,6 +166,16 @@ static const struct sunxi_ccu_desc sun8i_a83t_de2_clk_desc = {
.num_resets = ARRAY_SIZE(sun8i_a83t_de2_resets),
};
+static const struct sunxi_ccu_desc sun8i_h3_de2_clk_desc = {
+ .ccu_clks = sun8i_h3_de2_clks,
+ .num_ccu_clks = ARRAY_SIZE(sun8i_h3_de2_clks),
+
+ .hw_clks = &sun8i_h3_de2_hw_clks,
+
+ .resets = sun8i_a83t_de2_resets,
+ .num_resets = ARRAY_SIZE(sun8i_a83t_de2_resets),
+};
+
static const struct sunxi_ccu_desc sun50i_a64_de2_clk_desc = {
.ccu_clks = sun8i_a83t_de2_clks,
.num_ccu_clks = ARRAY_SIZE(sun8i_a83t_de2_clks),
@@ -237,6 +280,10 @@ static const struct of_device_id sunxi_de2_clk_ids[] = {
.compatible = "allwinner,sun8i-a83t-de2-clk",
.data = &sun8i_a83t_de2_clk_desc,
},
+ {
+ .compatible = "allwinner,sun8i-h3-de2-clk",
+ .data = &sun8i_h3_de2_clk_desc,
+ },
{
.compatible = "allwinner,sun8i-v3s-de2-clk",
.data = &sun8i_v3s_de2_clk_desc,
--
2.14.2
^ permalink raw reply related
* [PATCH v3 01/11] dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222122243.25735-1-icenowy@aosc.io>
The DE2 CCU is different on A83T and H3 -- the parent of the clocks on
A83T is PLL_DE but on H3 it's the DE module clock. This is not noticed
when I develop the DE2 CCU driver.
Fix the binding by using different compatibles for A83T and H3, adding
notes for the PLL_DE usage on A83T, and change the binding example's
compatible from A83T to H3 (as it specifies the DE module clock).
Fixes: ed74f8a8a679 ("dt-bindings: add binding for the Allwinner DE2 CCU")
Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
---
Documentation/devicetree/bindings/clock/sun8i-de2.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/sun8i-de2.txt b/Documentation/devicetree/bindings/clock/sun8i-de2.txt
index 631d27cd89d6..f2fa87c4765c 100644
--- a/Documentation/devicetree/bindings/clock/sun8i-de2.txt
+++ b/Documentation/devicetree/bindings/clock/sun8i-de2.txt
@@ -4,13 +4,14 @@ Allwinner Display Engine 2.0 Clock Control Binding
Required properties :
- compatible: must contain one of the following compatibles:
- "allwinner,sun8i-a83t-de2-clk"
+ - "allwinner,sun8i-h3-de2-clk"
- "allwinner,sun8i-v3s-de2-clk"
- "allwinner,sun50i-h5-de2-clk"
- reg: Must contain the registers base address and length
- clocks: phandle to the clocks feeding the display engine subsystem.
Three are needed:
- - "mod": the display engine module clock
+ - "mod": the display engine module clock (on A83T it's the DE PLL)
- "bus": the bus clock for the whole display engine subsystem
- clock-names: Must contain the clock names described just above
- resets: phandle to the reset control for the display engine subsystem.
@@ -19,7 +20,7 @@ Required properties :
Example:
de2_clocks: clock at 1000000 {
- compatible = "allwinner,sun8i-a83t-de2-clk";
+ compatible = "allwinner,sun8i-h3-de2-clk";
reg = <0x01000000 0x100000>;
clocks = <&ccu CLK_BUS_DE>,
<&ccu CLK_DE>;
--
2.14.2
^ permalink raw reply related
* [PATCH v3 00/11] Allwinner H3/H5/A64(DE2) SimpleFB support
From: Icenowy Zheng @ 2017-12-22 12:22 UTC (permalink / raw)
To: linux-arm-kernel
This patchset adds support for the SimpleFB on Allwinner SoCs with
"Display Engine 2.0".
PATCH 1 to PATCH 3 are DE2 CCU fixes for H3/H5 SoCs.
PATCH 4 adds the pipeline strings for DE2 SimpleFB.
PATCH 5 to 7 adds necessary device tree nodes (DE2 CCU and SimpleFB)
for H3/H5 SoCs.
PATCH 8 to 11 are for Allwinner A64 SoC to enable SimpleFB.
Icenowy Zheng (11):
dt-bindings: fix the binding of Allwinner DE2 CCU of A83T and H3
clk: sunxi-ng: add support for Allwinner H3 DE2 CCU
clk: sunxi-ng: fix the A64/H5 clock description of DE2 CCU
dt-bindings: simplefb-sunxi: add pipelines for DE2
ARM: sun8i: h3/h5: add DE2 CCU device node for H3
arm64: allwinner: h5: add compatible string for DE2 CCU
ARM: sunxi: h3/h5: add simplefb nodes
dt-bindings: add binding for A64 DE2 CCU SRAM
clk: sunxi-ng: add support for Allwinner A64 DE2 CCU
arm64: allwinner: a64: add DE2 CCU for A64 SoC
arm64: allwinner: a64: add simplefb for A64 SoC
.../devicetree/bindings/clock/sun8i-de2.txt | 10 ++-
.../bindings/display/simple-framebuffer-sunxi.txt | 4 +
arch/arm/boot/dts/sun8i-h3.dtsi | 4 +
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 43 +++++++++++
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 65 +++++++++++++++++
arch/arm64/boot/dts/allwinner/sun50i-h5.dtsi | 4 +
drivers/clk/sunxi-ng/ccu-sun8i-de2.c | 85 +++++++++++++++++++---
7 files changed, 202 insertions(+), 13 deletions(-)
--
2.14.2
^ permalink raw reply
* [PATCH] dmaengine: ti-dma-crossbar: Fix event mapping for TPCC_EVT_MUX_60_63
From: Vinod Koul @ 2017-12-22 12:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171219105116.24335-1-peter.ujfalusi@ti.com>
On Tue, Dec 19, 2017 at 12:51:16PM +0200, Peter Ujfalusi wrote:
> From: Vignesh R <vigneshr@ti.com>
>
> Register layout of a typical TPCC_EVT_MUX_M_N register is such that the
> lowest numbered event is at the lowest byte address and highest numbered
> event at highest byte address. But TPCC_EVT_MUX_60_63 register layout is
> different, in that the lowest numbered event is at the highest address
> and highest numbered event is at the lowest address. Therefore, modify
> ti_am335x_xbar_write() to handle TPCC_EVT_MUX_60_63 register
> accordingly.
Applied, thanks
--
~Vinod
^ permalink raw reply
* [PATCH v5 02/12] drm/panel: lvds: Add support for the power-supply property
From: Laurent Pinchart @ 2017-12-22 12:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0c0819bdf88fa948188df95e57a10820a8a4548d.1513854122.git-series.maxime.ripard@free-electrons.com>
Hi Maxime,
Thank you for the patch.
On Thursday, 21 December 2017 13:02:28 EET Maxime Ripard wrote:
> A significant number of panels need to power up a regulator in order to
> operate properly. Add support for the power-supply property to enable and
> disable such a regulator whenever needed.
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/gpu/drm/panel/panel-lvds.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-lvds.c
> b/drivers/gpu/drm/panel/panel-lvds.c index e2d57c01200b..57e38a9e7ab4
> 100644
> --- a/drivers/gpu/drm/panel/panel-lvds.c
> +++ b/drivers/gpu/drm/panel/panel-lvds.c
> @@ -17,6 +17,7 @@
> #include <linux/module.h>
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> #include <drm/drmP.h>
> @@ -39,6 +40,7 @@ struct panel_lvds {
> bool data_mirror;
>
> struct backlight_device *backlight;
> + struct regulator *supply;
>
> struct gpio_desc *enable_gpio;
> struct gpio_desc *reset_gpio;
> @@ -69,6 +71,9 @@ static int panel_lvds_unprepare(struct drm_panel *panel)
> if (lvds->enable_gpio)
> gpiod_set_value_cansleep(lvds->enable_gpio, 0);
>
> + if (lvds->supply)
> + regulator_disable(lvds->supply);
> +
> return 0;
> }
>
> @@ -76,6 +81,17 @@ static int panel_lvds_prepare(struct drm_panel *panel)
> {
> struct panel_lvds *lvds = to_panel_lvds(panel);
>
> + if (lvds->supply) {
> + int err;
> +
> + err = regulator_enable(lvds->supply);
> + if (err < 0) {
> + dev_err(lvds->dev, "failed to enable supply: %d\n",
> + err);
> + return err;
> + }
> + }
> +
> if (lvds->enable_gpio)
> gpiod_set_value_cansleep(lvds->enable_gpio, 1);
>
> @@ -196,6 +212,13 @@ static int panel_lvds_probe(struct platform_device
> *pdev) if (ret < 0)
> return ret;
>
> + lvds->supply = devm_regulator_get_optional(lvds->dev, "power");
> + if (IS_ERR(lvds->supply)) {
> + ret = PTR_ERR(lvds->supply);
> + dev_err(lvds->dev, "failed to request regulator: %d\n", ret);
> + return ret;
> + }
> +
> /* Get GPIOs and backlight controller. */
> lvds->enable_gpio = devm_gpiod_get_optional(lvds->dev, "enable",
> GPIOD_OUT_LOW);
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH v5 01/12] dt-bindings: panel: lvds: Document power-supply property
From: Laurent Pinchart @ 2017-12-22 12:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <0a6a3abcf1a6b7f0e66a81af8a44c5c0566ce06c.1513854122.git-series.maxime.ripard@free-electrons.com>
Hi Maxime,
Thank you for the patch.
On Thursday, 21 December 2017 13:02:27 EET Maxime Ripard wrote:
> The power-supply property is used by a vast majority of panels, including
> panel-simple. Let's document it as a common property
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
> Documentation/devicetree/bindings/display/panel/panel-common.txt | 6 ++++++
> Documentation/devicetree/bindings/display/panel/panel-lvds.txt | 1 +
> Documentation/devicetree/bindings/display/panel/simple-panel.txt | 2 +- 3
> files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/panel-common.txt
> b/Documentation/devicetree/bindings/display/panel/panel-common.txt index
> ec52c472c845..125ea68052af 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-common.txt
> +++ b/Documentation/devicetree/bindings/display/panel/panel-common.txt
> @@ -78,6 +78,12 @@ used for panels that implement compatible control
> signals. while active. Active high reset signals can be supported by
> inverting the GPIO specifier polarity flag.
>
> +Power
> +-----
> +
> +- power-supply: many display panels need an additional power supply in
> + order to be fully powered-up. For such panels, power-supply contains
> + a phandle to the regulator powering the panel.
I think we should give more details here about the limitations of this
property. How about the following explanation ?
- power-supply: display panels require power to be supplied. While several
panels need more than one power supply with panel-specific constraints
governing the order and timings of the power supplies, in many cases a single
power supply is sufficient, either because the panel has a single power rail,
or because all its power rails can be driven by the same supply. In that case
the power-supply property specifies the supply powering the panel as a phandle
to a regulator.
> Backlight
> ---------
> diff --git a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt index
> b938269f841e..250850a2150b 100644
> --- a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> @@ -32,6 +32,7 @@ Optional properties:
> - label: See panel-common.txt.
> - gpios: See panel-common.txt.
> - backlight: See panel-common.txt.
> +- power-supply: See panel-common.txt.
> - data-mirror: If set, reverse the bit order described in the data mappings
> below on all data lanes, transmitting bits for slots 6 to 0 instead of 0 to
> 6.
> diff --git
> a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> b/Documentation/devicetree/bindings/display/panel/simple-panel.txt index
> 1341bbf4aa3d..16d8ff088b7d 100644
> --- a/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> +++ b/Documentation/devicetree/bindings/display/panel/simple-panel.txt
> @@ -1,7 +1,7 @@
> Simple display panel
>
> Required properties:
> -- power-supply: regulator to provide the supply voltage
> +- power-supply: See panel-common.txt
>
> Optional properties:
> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing
--
Regards,
Laurent Pinchart
^ permalink raw reply
* AW: [PATCH v2 4/5] ARM: dts: Add support for emtrion emCON-MX6 series
From: Türk, Jan @ 2017-12-22 11:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222105642.GD18255@piout.net>
Hi,
> On 22/12/2017 at 11:43:33 +0100, Andreas F?rber wrote:
> > >> I'll change it for v3 of this patch however it will end up like this:
> > >> //SPDX-License...
> > >
> > > That should be /* SPDX-License */, // is for c files.
> >
> > Got any reference for that? Since we're using the C preprocessor
> > before feeding them to dtc, we can use the same // style for both, builds fine.
> >
> > Only for my private DT overlay files that I use directly with dtc I
> > couldn't adopt that style.
> >
> The doc states:
>
> If a specific tool cannot handle the standard comment style, then the
> appropriate comment mechanism which the tool accepts shall be used. This is
> the reason for having the "/\* \*/" style comment in C header files.
>
> I interpreted that as dtc doesn't handle // comments, use /**/
>
> But I agree it also states:
> .dts{i}: // SPDX-License-Identifier: <SPDX License Expression>
>
> So I think we will end up with a mix of both.
>
after some regexp on arch/arm/boot/dts, the current state is:
216 SPDX identifiers total
184 by GregKH in b24413180f5600bcb3bb70fbed5cf186b60864bd starting with //
2 times /* also by GregKH in the commit above (in .h files)
27 occurrences of "* SPDX"
However, the de-facto comment style in the arm devicetrees seems to be /* */
So with the current information I would prepare v3 with:
// SPDX-License-Identifier
/* Copyright-text + Header
*/
[...]
Jan
---
emtrion GmbH
Alter Schlachthof 45
76131 Karlsruhe
GERMANY
https://www.emtrion.de
Amtsgericht Mannheim
HRB 110 300
Gesch?ftsf?hrer: Dieter Baur, Ramona Maurer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: winmail.dat
Type: application/ms-tnef
Size: 5618 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20171222/45969992/attachment-0001.bin>
^ permalink raw reply
* [PATCH V1 0/1] Fix kernel panic caused by device ID duplication presented to the IOMMU
From: Tomasz Nowicki @ 2017-12-22 11:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7f7fca76-88f8-6ee7-c402-fe4300c62253@arm.com>
Hi Robin,
On 19.12.2017 17:34, Robin Murphy wrote:
> Hi Tomasz,
>
> On 19/12/17 15:13, Tomasz Nowicki wrote:
>> Here is my lspci output of ThunderX2 for which I am observing kernel
>> panic coming from
>> SMMUv3 driver -> arm_smmu_write_strtab_ent() -> BUG_ON(ste_live):
>>
>> # lspci -vt
>> -[0000:00]-+-00.0-[01-1f]--+ [...]
>> ??????????????????????????? + [...]
>> ??????????????????????????? \-00.0-[1e-1f]----00.0-[1f]----00.0
>> ASPEED Technology, Inc. ASPEED Graphics Family
>>
>> ASP device -> 1f:00.0 VGA compatible controller: ASPEED Technology,
>> Inc. ASPEED Graphics Family
>> PCI-Express to PCI/PCI-X Bridge -> 1e:00.0 PCI bridge: ASPEED
>> Technology, Inc. AST1150 PCI-to-PCI Bridge
>> While setting up ASP device SID in IORT dirver:
>> iort_iommu_configure() -> pci_for_each_dma_alias()
>> we need to walk up and iterate over each device which alias
>> transaction from
>> downstream devices.
>>
>> AST device (1f:00.0) gets BDF=0x1f00 and corresponding SID=0x1f00 from
>> IORT.
>> Bridge (1e:00.0) is the first alias. Following PCI Express to
>> PCI/PCI-X Bridge
>> spec: PCIe-to-PCI/X bridges alias transactions from downstream devices
>> using
>> the subordinate bus number. For bridge (1e:00.0), the subordinate is
>> equal
>> to 0x1f. This gives BDF=0x1f00 and SID=1f00 which is the same as
>> downstream
>> device. So it is possible to have two identical SIDs. The question is
>> what we
>> should do about such case. Presented patch prevents from registering
>> the same
>> ID so that SMMUv3 is not complaining later on.
>
> Ooh, subtle :( There is logic in arm_smmu_attach_device() to tolerate
> grouped devices aliasing to the same ID, but I guess I overlooked the
> distinction of a device sharing an alias ID with itself. I'm not sure
> I really like trying to work around this in generic code, since
> fwspec->ids is essentially opaque data in a driver-specific format - in
> theory a driver is free to encode a single logical ID into multiple
> fwspec elements (I think I did that in an early draft of SMMUv2 SMR
> support), at which point this approach might corrupt things massively.
>
> Does the (untested) diff below suffice?
>
> Robin.
>
> ----->8-----diff --git a/drivers/iommu/arm-smmu-v3.c
> b/drivers/iommu/arm-smmu-v3.c
> index f122071688fd..d8a730d83401 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -1731,7 +1731,7 @@ static __le64 *arm_smmu_get_step_for_sid(struct
> arm_smmu_device *smmu, u32 sid)
>
> ?static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
> ?{
> -??? int i;
> +??? int i, j;
> ???? struct arm_smmu_master_data *master = fwspec->iommu_priv;
> ???? struct arm_smmu_device *smmu = master->smmu;
>
> @@ -1739,6 +1739,13 @@ static void arm_smmu_install_ste_for_dev(struct
> iommu_fwspec *fwspec)
> ???????? u32 sid = fwspec->ids[i];
> ???????? __le64 *step = arm_smmu_get_step_for_sid(smmu, sid);
>
> +??????? /* Bridged PCI devices may end up with duplicated IDs */
> +??????? for (j = 0; j < i; j++)
> +??????????? if (fwspec->ids[j] == sid)
> +??????????????? break;
> +??????? if (j < i)
> +??????????? continue;
> +
> ???????? arm_smmu_write_strtab_ent(smmu, sid, step, &master->ste);
> ???? }
> ?}
The patch works for me:
Tested-by: Tomasz Nowicki <Tomasz.Nowicki@cavium.com>
The bug causes a regression on our platform and would be nice to fix it
for 4.15. Since no more comments so far, IMO we can put the fix to
SMMUv3 driver. Do you prefer to send patch yourself or would you like me
to do it?
Thanks,
Tomasz
^ permalink raw reply
* [PATCH 4/4] ARM: dts: vf610-zii-dev-rev-b: add interrupts for 88e1545 PHY
From: Russell King - ARM Linux @ 2017-12-22 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4322b18f-636f-6904-4d43-3860681050af@gmail.com>
On Thu, Dec 21, 2017 at 04:20:34PM -0800, Florian Fainelli wrote:
> On 12/21/2017 04:14 PM, Russell King - ARM Linux wrote:
> > On Thu, Dec 21, 2017 at 11:53:47PM +0100, Linus Walleij wrote:
> >> On Thu, Dec 21, 2017 at 6:32 PM, Russell King - ARM Linux
> >> <linux@armlinux.org.uk> wrote:
> >>
> >>> What we have here is _really_ a shared interrupt between four
> >>> separate devices, and we need a way to sanely describe resources
> >>> shared between several device instances to pinmux. Unfortunately,
> >>> it seems pinmux is designed around one device having exclusive use
> >>> of a resource, which makes it hard to describe shared interrupts in
> >>> DT.
> >>>
> >>> Given that DT should be a description of the hardware, and should be
> >>> independent of the OS implementation, I'd say this is a pinmux bug,
> >>> because pinmux gets in the way of describing the hardware correctly.
> >>> ;)
> >>
> >> Hm that would be annoying. But when I look at it I think it would
> >> actually work. Did you try just assigning the same pin control
> >> state to all the PHY's and see what happens?
> >>
> >> Just set
> >> pinctrl-names = "default";
> >> pinctrl-0 = <&pinctrl_mv88e1545>;
> >>
> >> on all of them?
> >
> > It was tried, DT was happy, but the kernel on boot complained because
> > pinctrl objected, which caused the drivers to fail to bind:
> >
> > libphy: mdio: probed
> > vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:01
> > vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:01) status -22
> > vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
> > Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:01: Error applying setting, reverse things back
> > Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:01 failed with error -22
> > vf610-pinctrl 40048000.iomuxc: pin VF610_PAD_PTB0 already requested by !mdio-mux!mdio at 4!switch at 0!mdio:00; cannot claim for !mdio-mux!mdio at 4!switch at 0!mdio:02
> > vf610-pinctrl 40048000.iomuxc: pin-22 (!mdio-mux!mdio at 4!switch at 0!mdio:02) status -22
> > vf610-pinctrl 40048000.iomuxc: could not request pin 22 (VF610_PAD_PTB0) from group pinctrl-mv88e1545 on device 40048000.iomuxc
> > Marvell 88E1545 !mdio-mux!mdio at 4!switch at 0!mdio:02: Error applying setting, reverse things back
> > Marvell 88E1545: probe of !mdio-mux!mdio at 4!switch at 0!mdio:02 failed with error -22
> >
>
> You could also see it another way, because this is a quad PHY in a
> single package, you could theoretically have a representation that
> exposes a node container for the 4 PHYs, and that container node
> requests the pinmux/pinctrl. Of course, this would not work with the
> MDIO code which would not go one level down, and would expect the PHYs
> to be at the same level as the container node...
It would actually - we have other devices that sit on buses that take
several addresses, and we describe the "first" main device or use MFD
for it.
For example, in the case of the TDA998x HDMI encoder, these are two
devices merged into one package - the HDMI encoder at one address, and
a TDA9950 at another address. Both addresses are related, so if you
tie the address configuration pins, the offset is added to both base
addresses. We represent the TDA998x in DT, and have the TDA998x
driver create a separate device itself for the TDA9950.
What we could do for any multi-package PHY is describe the first PHY
as a multi-package PHY in DT, extend the phy binding to include a PHY
package index, and have the PHY driver create the MDIO devices for
the other PHYs. Eg,
switch {
...
ports {
port at 0 {
reg = <0>;
label = "lan6";
phy-handle = <&switch2phy 0>;
};
port at 1 {
reg = <1>;
label = "lan6";
phy-handle = <&switch2phy 1>;
};
port at 2 {
reg = <2>;
label = "lan6";
phy-handle = <&switch2phy 2>;
};
};
mdio {
#address-cells = <1>;
#size-cells = <0>;
switch2phy: phy at 0 {
compatible = "marvell,88e1545", "ethernet-phy-ieee802.3-c22";
interrupt-parent = <&gpio0>;
interrupts = <22 IRQ_TYPE_LEVEL_LOW>;
reg = <0>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_mv88e1545>;
};
};
};
The "marvell,88e1545" driver would be responsible for creating the
PHY devices for the other MDIO bus addresses (iow 1 to 3.)
This would be an accurate respresentation of the hardware in DT,
probably more so than the trap we seem to have fallen into by
describing the individual PHYs - which we've fallen into because
that's how our current implementation requires us to describe them.
Since DT is supposed to be a hardware description, I think the question
we ought to ask is: if we were starting afresh, how would we describe
these packages that contain multiple PHYs?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* [PATCH v6 06/11] thermal: armada: Add support for Armada AP806
From: Baruch Siach @ 2017-12-22 11:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171222114948.32c36369@xps13>
Hi Miqu?l,
On Fri, Dec 22, 2017 at 11:49:48AM +0100, Miquel RAYNAL wrote:
> On Fri, 22 Dec 2017 12:14:26 +0200
> Baruch Siach <baruch@tkos.co.il> wrote:
> > On Fri, Dec 22, 2017 at 10:32:21AM +0100, Miquel Raynal wrote:
> > > From: Baruch Siach <baruch@tkos.co.il>
> > >
> > > The AP806 component is integrated in the Armada 8K and 7K lines of
> > > processors.
> > >
> > > The thermal sensor sample field on the status register is a signed
> > > value. Extend armada_get_temp() and the driver structure to handle
> > > signed values.
> > >
> > > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> > > [<miquel.raynal@free-electrons.com>: Changes when applying over the
> > > previous patches, including the register names changes, also
> > > switched the coefficients values to s64 instead of unsigned long to
> > > deal with negative values and used do_div instead of the
> > > traditionnal '/'] Signed-off-by: Miquel Raynal
> > > <miquel.raynal@free-electrons.com> Reviewed-by: Gregory CLEMENT
> > > <gregory.clement@free-electrons.com> Tested-by: Gregory CLEMENT
> > > <gregory.clement@free-electrons.com> ---
> >
> > [..]
> >
> > > static int armada_get_temp(struct thermal_zone_device *thermal,
> > > - int *temp)
> > > + int *temperature)
> > > {
> > > struct armada_thermal_priv *priv = thermal->devdata;
> > > - unsigned long reg;
> > > - unsigned long m, b, div;
> > > + u32 reg, div;
> > > + s64 sample, b, m;
> > > + u64 tmp;
> > >
> > > /* Valid check */
> > > if (priv->data->is_valid && !priv->data->is_valid(priv)) {
> > > @@ -178,6 +197,11 @@ static int armada_get_temp(struct
> > > thermal_zone_device *thermal,
> > > reg = readl_relaxed(priv->status);
> > > reg = (reg >> priv->data->temp_shift) &
> > > priv->data->temp_mask;
> > > + if (priv->data->signed_sample)
> > > + /* The most significant bit is the sign bit */
> > > + sample = sign_extend32(reg,
> > > fls(priv->data->temp_mask) - 1);
> > > + else
> > > + sample = reg;
> > >
> > > /* Get formula coeficients */
> > > b = priv->data->coef_b;
> > > @@ -185,9 +209,13 @@ static int armada_get_temp(struct
> > > thermal_zone_device *thermal, div = priv->data->coef_div;
> > >
> > > if (priv->data->inverted)
> > > - *temp = ((m * reg) - b) / div;
> > > + tmp = (m * sample) - b;
> > > else
> > > - *temp = (b - (m * reg)) / div;
> > > + tmp = b - (m * sample);
> > > +
> > > + do_div(tmp, div);
> > > + *temperature = (int)tmp;
> >
> > Nitpick: why not (untested)
> >
> > #include <linux/math64.h>
> >
> > if (priv->data->inverted)
> > *temp = div_s64((m * sample) - b, div);
> > else
> > *temp = div_s64(b - (m * sample), div);
>
> Indeed I could also use div_s64, but the result must be unsigned anyway.
*temp is signed, as far as I can see. Temperature can go below zero, at least
in the Celsius scale.
> But this does all the operations on the same line, maybe this is more
> readable, I will update it and send (hopefully) the last version :)
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* [PATCH v3 6/6] ARM: imx_v6_v7_defconfig: Enable Dialog Semiconductor DA9062 driver
From: Stefan Riedmueller @ 2017-12-22 10:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1513940353-6145-1-git-send-email-s.riedmueller@phytec.de>
The phyCORE-i.MX 6 uses the DA9062/63 PMIC, RTC and Watchdog driver.
Enable these options by default.
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
arch/arm/configs/imx_v6_v7_defconfig | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index 6726c83..e3c4163 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -216,8 +216,10 @@ CONFIG_THERMAL_WRITABLE_TRIPS=y
CONFIG_CPU_THERMAL=y
CONFIG_IMX_THERMAL=y
CONFIG_WATCHDOG=y
+CONFIG_DA9062_WATCHDOG=y
CONFIG_IMX2_WDT=y
CONFIG_MFD_DA9052_I2C=y
+CONFIG_MFD_DA9062=y
CONFIG_MFD_MC13XXX_SPI=y
CONFIG_MFD_MC13XXX_I2C=y
CONFIG_MFD_STMPE=y
@@ -225,6 +227,7 @@ CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
CONFIG_REGULATOR_ANATOP=y
CONFIG_REGULATOR_DA9052=y
+CONFIG_REGULATOR_DA9062=y
CONFIG_REGULATOR_GPIO=y
CONFIG_REGULATOR_MC13783=y
CONFIG_REGULATOR_MC13892=y
@@ -348,6 +351,7 @@ CONFIG_RTC_DRV_ISL1208=y
CONFIG_RTC_DRV_PCF8523=y
CONFIG_RTC_DRV_PCF8563=y
CONFIG_RTC_DRV_M41T80=y
+CONFIG_RTC_DRV_DA9063=y
CONFIG_RTC_DRV_MC13XXX=y
CONFIG_RTC_DRV_MXC=y
CONFIG_RTC_DRV_SNVS=y
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox