Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] dt-bindings: chosen: Add clocksource and clockevent selection
From: Boris Brezillon @ 2017-12-14 20:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Alexandre Belloni, Mark Rutland, devicetree@vger.kernel.org,
	Daniel Lezcano, linux-kernel@vger.kernel.org, Thomas Gleixner,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAL_JsqLm5n6s=cr2hBZp4H9pDPKHUOPWs94JMWYFYJ3hsMMxZQ@mail.gmail.com>

Hi Rob,

On Wed, 13 Dec 2017 16:57:50 -0600
Rob Herring <robh+dt@kernel.org> wrote:

> On Wed, Dec 13, 2017 at 12:53 PM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
> > The clocksource and clockevent timer are probed early in the boot process.
> > At that time it is difficult for linux to know whether a particular timer
> > can be used as the clocksource or the clockevent or by another driver,
> > especially when they are all identical or have similar features.  
> 
> If all identical, then it shouldn't matter. "similar" means some
> difference. Describe those differences.

We had this discussion already. Those timers might be connected to
external pins and may serve the role of PWM generators or capture
devices. We can also chain timers and provide a clocksource with a
better resolution or one that wraps less often. In the end it's all
about how the user plans to use its system, and this has to be
described somehow. Assuming that the software can decide by itself
which timer should be used or how many of them should be chained is
just an utopia.

> 
> > Until now, multiple strategies have been used to solve that:
> >  - use Kconfig option as MXC_USE_EPIT or ATMEL_TCB_CLKSRC_BLOCK  
> 
> Compile time probably means only one option is really used.

Compile time selection prevents using the same kernel for different
boards (in other words, no multi-v7), so not really an option here.

> 
> >  - use a kernel parameter as the "clocksource" early_param in mach-omap2  
> 
> Yeah, OMAP was one of the previous times this came up and also
> attempted something like this. This parameter predates selecting
> timers based on features described in DT. Look at commit
> 2eb03937df3ebc (ARM: OMAP3: Update clocksource timer selection).

Then, would you accept to have a property saying that a specific timer
is a free-running timer (suitable for clocksource) or a programmable
timer (suitable for clkevent)? Or are you saying that you don't like the
way timers are differentiated on omap?

> 
> >  - registering the first seen timer as a clockevent and the second one as
> >  a clocksource as in rk_timer_init or dw_apb_timer_init
> >
> > Add a linux,clocksource and a linux,clockevent node in chosen with a timer
> > property pointing to the timer to use. Other properties, like the targeted
> > precision may be added later.  
> 
> Open ended expansion of this does not help convince me it is needed.

It's not an open ended expansion, we're just trying to find a way to
describe which timer blocks should be used as free running timers
(clksource) and which one should be used as programmable timers
(clkevent). Automatically selecting timer blocks to assign to the
clkevent or clocksource is not so easy (as has been explained earlier)
because at the time the system registers its clksource/clkevent devices
we might not have all the necessary information to know which timer
blocks will be reserved for other usage later on. The use case I have
in mind is DT overlays, where one of the overlay is using a timer as a
PWM generator. If the clkevent or clksource has already claimed the
timer connected to the pins the overlay is using, then we're screwed,
and there's no way the system can know that ahead of time except by
pre-assigning a timer to the clksource or clkevent feature.

So really, we need a way to assign a specific timer to a specific
feature. You've refused the different proposals we made so far, so
what's your alternative? I mean a real alternative that solve the "an
auto-selected timer might be claimed by another driver at some point"
problem.

Thanks,

Boris

^ permalink raw reply

* Re: [PATCH v2 6/7] i3c: master: Add driver for Cadence IP
From: Randy Dunlap @ 2017-12-14 19:54 UTC (permalink / raw)
  To: Boris Brezillon, Wolfram Sang, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Jonathan Corbet, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	Greg Kroah-Hartman, Arnd Bergmann
  Cc: Przemyslaw Sroka, Arkadiusz Golec, Alan Douglas, Bartosz Folta,
	Damian Kos, Alicja Jurasik-Urbaniak, Cyprian Wronka,
	Suresh Punnoose, Thomas Petazzoni, Nishanth Menon, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vitor Soares,
	Geert Uytterhoeven, Linus Walleij
In-Reply-To: <20171214151610.19153-7-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

On 12/14/2017 07:16 AM, Boris Brezillon wrote:
> Add a driver for Cadence I3C master IP.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> Changes in v2:
> - Add basic IBI support. Note that the IP is not really reliable with
>   regards to IBI because you can't extract IBI payloads as soon as you
>   have more than one IBI waiting in the HW queue. This is something
>   that will hopefully be addressed in future revisions of this IP
> - Add a simple xfer queueing mechanism to optimize message queuing.
> - Fix a few bugs
> - Add support for Hot Join
> ---
>  drivers/i3c/master/Kconfig           |    5 +
>  drivers/i3c/master/Makefile          |    1 +
>  drivers/i3c/master/i3c-master-cdns.c | 1797 ++++++++++++++++++++++++++++++++++
>  3 files changed, 1803 insertions(+)
>  create mode 100644 drivers/i3c/master/i3c-master-cdns.c
> 
> diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> index e69de29bb2d1..56b9a18543b2 100644
> --- a/drivers/i3c/master/Kconfig
> +++ b/drivers/i3c/master/Kconfig
> @@ -0,0 +1,5 @@
> +config CDNS_I3C_MASTER
> +	tristate "Cadence I3C master driver"
> +	depends on I3C
> +	help
> +	  Enable this driver if you want to support Cadence I3C master block.
> diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> index e69de29bb2d1..4c4304aa9534 100644
> --- a/drivers/i3c/master/Makefile
> +++ b/drivers/i3c/master/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_CDNS_I3C_MASTER)		+= i3c-master-cdns.o
> diff --git a/drivers/i3c/master/i3c-master-cdns.c b/drivers/i3c/master/i3c-master-cdns.c
> new file mode 100644
> index 000000000000..3e3ef37c01c2
> --- /dev/null
> +++ b/drivers/i3c/master/i3c-master-cdns.c
> @@ -0,0 +1,1797 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2017 Cadence Design Systems Inc.
> + *
> + * Author: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + */
> +

Rule #1 from submit-checklist.rst:
1) If you use a facility then #include the file that defines/declares
   that facility.  Don't depend on other header files pulling in ones
   that you use.


#include <linux/bitops.h>
for BIT(x) and hweight8() and clear_bit() and find_next_bit()
and hweight32()

> +#include <linux/clk.h>

#include <linux/err.h>
for IS_ERR(), PTR_ERR()

#include <linux/errno.h>
for error codes

> +#include <linux/i3c/master.h>
> +#include <linux/interrupt.h>

#include <linux/io.h>
for writel()

> +#include <linux/iopoll.h>

#include <linux/ioport.h>
for IORESOURCE_MEM

#include <linux/kernel.h>
for DIV_ROUND_UP()

#include <linux/list.h>
for list() macros and INIT_LIST_HEAD()

> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>

#include <linux/spinlock.h>

#include <linux/workqueue.h>
for workqueue functions and INIT_WORK()

> +
> +#define DEV_ID				0x0
> +#define DEV_ID_I3C_MASTER		0x5034

[snip]



> +#define I3C_DDR_FIRST_DATA_WORD_PREAMBLE	0x2
> +#define I3C_DDR_DATA_WORD_PREAMBLE		0x3
> +
> +#define I3C_DDR_PREAMBLE(p)			((p) << 18)
> +
> +static u32 prepare_ddr_word(u16 payload)
> +{
> +	u32 ret;
> +	u16 pb;
> +
> +	ret = (u32)payload << 2;
> +
> +	/* Calculate parity. */
> +	pb = (payload >> 15) ^ (payload >> 13) ^ (payload >> 11) ^
> +	     (payload >> 9) ^ (payload >> 7) ^ (payload >> 5) ^
> +	     (payload >> 3) ^ (payload >> 1);
> +	ret |= (pb & 1) << 1;
> +	pb = (payload >> 14) ^ (payload >> 12) ^ (payload >> 10) ^
> +	     (payload >> 8) ^ (payload >> 6) ^ (payload >> 4) ^
> +	     (payload >> 2) ^ payload ^ 1;
> +	ret |= (pb & 1);
> +
> +	return ret;
> +}
> +
> +static u32 prepare_ddr_data_word(u16 data, bool first)
> +{
> +	return prepare_ddr_word(data) | I3C_DDR_PREAMBLE(first ? 2 : 3);

Just defined macros for 2 & 3 above. Use them instead of magic numbers?

> +}
> +
> +#define I3C_DDR_READ_CMD	BIT(15)
> +
> +static u32 prepare_ddr_cmd_word(u16 cmd)
> +{
> +	return prepare_ddr_word(cmd) | I3C_DDR_PREAMBLE(1);
> +}
> +
> +static u32 prepare_ddr_crc_word(u8 crc5)
> +{
> +	return (((u32)crc5 & 0x1f) << 9) | (0xc << 14) |
> +	       I3C_DDR_PREAMBLE(1);
> +}
> +
> +static u8 update_crc5(u8 crc5, u16 word)
> +{
> +	u8 crc0;
> +	int i;
> +
> +	/*
> +	 * crc0 = next_data_bit ^ crc[4]
> +	 *                1         2            3       4
> +	 * crc[4:0] = { crc[3:2], crc[1]^crc0, crc[0], crc0 }
> +	 */
> +	for (i = 0; i < 16; ++i) {
> +		crc0 = ((word >> (15 - i)) ^ (crc5 >> 4)) & 0x1;
> +		crc5 = ((crc5 << 1) & (0x18 | 0x2)) |
> +		       (((crc5 >> 1) ^ crc0) << 2) | crc0;
> +	}
> +
> +	return crc5 & 0x1F;
> +}
> +

[snip]



> +static int cdns_i3c_master_do_daa_locked(struct cdns_i3c_master *master)
> +{
> +	unsigned long i3c_lim_period, pres_step, i3c_scl_lim;
> +	struct i3c_device_info devinfo;
> +	struct i3c_device *i3cdev;
> +	u32 prescl1, ctrl, devs;
> +	int ret, slot, ncycles;
> +
> +	ret = i3c_master_entdaa_locked(&master->base);
> +	if (ret)
> +		return ret;
> +
> +	/* Now, add discovered devices to the bus. */
> +	i3c_scl_lim = master->i3c_scl_lim;
> +	devs = readl(master->regs + DEVS_CTRL);
> +	for (slot = find_next_bit(&master->free_dev_slots, BITS_PER_LONG, 1);
> +	     slot < BITS_PER_LONG;
> +	     slot = find_next_bit(&master->free_dev_slots,
> +				  BITS_PER_LONG, slot + 1)) {
> +		struct cdns_i3c_i2c_dev_data *data;
> +		u32 rr, max_fscl = 0;
> +		u8 addr;
> +
> +		if (!(devs & DEVS_CTRL_DEV_ACTIVE(slot)))
> +			continue;
> +
> +		data = kzalloc(sizeof(*data), GFP_KERNEL);
> +		if (!data)
> +			return -ENOMEM;
> +
> +		data->ibi = -1;
> +		data->id = slot;
> +		rr = readl(master->regs + DEV_ID_RR0(slot));
> +		addr = DEV_ID_RR0_GET_DEV_ADDR(rr);
> +		i3cdev = i3c_master_add_i3c_dev_locked(&master->base, addr);
> +		if (IS_ERR(i3cdev))
> +			return PTR_ERR(i3cdev);
> +
> +		i3c_device_get_info(i3cdev, &devinfo);
> +		clear_bit(data->id, &master->free_dev_slots);
> +		i3c_device_set_master_data(i3cdev, data);
> +
> +		max_fscl = max(I3C_CCC_MAX_SDR_FSCL(devinfo.max_read_ds),
> +			       I3C_CCC_MAX_SDR_FSCL(devinfo.max_write_ds));
> +		switch (max_fscl) {
> +		case I3C_SDR_DR_FSCL_8MHZ:
> +			max_fscl = 8000000;
> +			break;
> +		case I3C_SDR_DR_FSCL_6MHZ:
> +			max_fscl = 6000000;
> +			break;
> +		case I3C_SDR_DR_FSCL_4MHZ:
> +			max_fscl = 4000000;
> +			break;
> +		case I3C_SDR_DR_FSCL_2MHZ:
> +			max_fscl = 2000000;
> +			break;
> +		case I3C_SDR_DR_FSCL_MAX:
> +		default:
> +			max_fscl = 0;
> +			break;
> +		}
> +
> +		if (max_fscl && (max_fscl < i3c_scl_lim || !i3c_scl_lim))
> +			i3c_scl_lim = max_fscl;
> +	}
> +
> +	i3c_master_defslvs_locked(&master->base);
> +
> +	pres_step = 1000000000 / (master->base.bus->scl_rate.i3c * 4);

Does that build OK on 32-bit target arch?

> +
> +	/* No bus limitation to apply, bail out. */
> +	if (!i3c_scl_lim ||
> +	    (master->i3c_scl_lim && master->i3c_scl_lim <= i3c_scl_lim))
> +		return 0;
> +
> +	/* Configure PP_LOW to meet I3C slave limitations. */
> +	prescl1 = readl(master->regs + PRESCL_CTRL1) &
> +		  ~PRESCL_CTRL1_PP_LOW_MASK;
> +	ctrl = readl(master->regs + CTRL) & ~CTRL_DEV_EN;
> +
> +	i3c_lim_period = DIV_ROUND_UP(1000000000, i3c_scl_lim);
> +	ncycles = DIV_ROUND_UP(i3c_lim_period, pres_step) - 4;
> +	if (ncycles < 0)
> +		ncycles = 0;
> +	prescl1 |= PRESCL_CTRL1_PP_LOW(ncycles);
> +
> +	/* Disable I3C master before updating PRESCL_CTRL1. */
> +	ret = cdns_i3c_master_disable(master);
> +	if (!ret) {
> +		writel(prescl1, master->regs + PRESCL_CTRL1);
> +		master->i3c_scl_lim = i3c_scl_lim;
> +	}
> +	cdns_i3c_master_enable(master);
> +
> +	return ret;
> +}
> +
> +static int cdns_i3c_master_bus_init(struct i3c_master_controller *m)
> +{
> +	struct cdns_i3c_master *master = to_cdns_i3c_master(m);
> +	unsigned long pres_step, sysclk_rate, max_i2cfreq;
> +	u32 ctrl, prescl0, prescl1, pres, low;
> +	struct i3c_device_info info = { };
> +	struct i3c_ccc_events events;
> +	struct i2c_device *i2cdev;
> +	struct i3c_device *i3cdev;
> +	int ret, slot, ncycles;
> +	u8 last_addr = 0;
> +
> +	switch (m->bus->mode) {
> +	case I3C_BUS_MODE_PURE:
> +		ctrl = CTRL_PURE_BUS_MODE;
> +		break;
> +
> +	case I3C_BUS_MODE_MIXED_FAST:
> +		ctrl = CTRL_MIXED_FAST_BUS_MODE;
> +		break;
> +
> +	case I3C_BUS_MODE_MIXED_SLOW:
> +		ctrl = CTRL_MIXED_SLOW_BUS_MODE;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	sysclk_rate = clk_get_rate(master->sysclk);
> +	if (!sysclk_rate)
> +		return -EINVAL;
> +
> +	pres = DIV_ROUND_UP(sysclk_rate, (m->bus->scl_rate.i3c * 4)) - 1;
> +	if (pres > PRESCL_CTRL0_MAX)
> +		return -ERANGE;
> +
> +	m->bus->scl_rate.i3c = sysclk_rate / ((pres + 1) * 4);
> +
> +	prescl0 = PRESCL_CTRL0_I3C(pres);
> +
> +	low = ((I3C_BUS_TLOW_OD_MIN_NS * sysclk_rate) / (pres + 1)) - 2;
> +	prescl1 = PRESCL_CTRL1_OD_LOW(low);
> +
> +	max_i2cfreq = m->bus->scl_rate.i2c;
> +
> +	pres = (sysclk_rate / (max_i2cfreq * 5)) - 1;
> +	if (pres > PRESCL_CTRL0_MAX)
> +		return -ERANGE;
> +
> +	m->bus->scl_rate.i2c = sysclk_rate / ((pres + 1) * 5);
> +
> +	prescl0 |= PRESCL_CTRL0_I2C(pres);
> +
> +	writel(DEVS_CTRL_DEV_CLR_ALL, master->regs + DEVS_CTRL);
> +
> +	i3c_bus_for_each_i2cdev(m->bus, i2cdev) {
> +		ret = cdns_i3c_master_attach_i2c_dev(master, i2cdev);
> +		if (ret)
> +			goto err_detach_devs;
> +	}
> +
> +	writel(prescl0, master->regs + PRESCL_CTRL0);
> +
> +	/* Calculate OD and PP low. */
> +	pres_step = 1000000000 / (m->bus->scl_rate.i3c * 4);
> +	ncycles = DIV_ROUND_UP(I3C_BUS_TLOW_OD_MIN_NS, pres_step) - 2;
> +	if (ncycles < 0)
> +		ncycles = 0;
> +	prescl1 = PRESCL_CTRL1_OD_LOW(ncycles);
> +	writel(prescl1, master->regs + PRESCL_CTRL1);
> +
> +	i3c_bus_for_each_i3cdev(m->bus, i3cdev) {
> +		ret = cdns_i3c_master_attach_i3c_dev(master, i3cdev);
> +		if (ret)
> +			goto err_detach_devs;
> +	}
> +
> +	/* Get an address for the master. */
> +	ret = i3c_master_get_free_addr(m, 0);
> +	if (ret < 0)
> +		goto err_detach_devs;
> +
> +	writel(prepare_rr0_dev_address(ret) | DEV_ID_RR0_IS_I3C,
> +	       master->regs + DEV_ID_RR0(0));
> +
> +	cdns_i3c_master_dev_rr_to_info(master, 0, &info);
> +	if (info.bcr & I3C_BCR_HDR_CAP)
> +		info.hdr_cap = I3C_CCC_HDR_MODE(I3C_HDR_DDR);
> +
> +	ret = i3c_master_set_info(&master->base, &info);
> +	if (ret)
> +		goto err_detach_devs;
> +
> +	/* Prepare RR slots before lauching DAA. */

	                           launching

> +	for (slot = find_next_bit(&master->free_dev_slots, BITS_PER_LONG, 1);
> +	     slot < BITS_PER_LONG;
> +	     slot = find_next_bit(&master->free_dev_slots,
> +				  BITS_PER_LONG, slot + 1)) {
> +		ret = i3c_master_get_free_addr(m, last_addr + 1);
> +		if (ret < 0)
> +			goto err_disable_master;
> +
> +		last_addr = ret;
> +		writel(prepare_rr0_dev_address(last_addr) | DEV_ID_RR0_IS_I3C,
> +		       master->regs + DEV_ID_RR0(slot));
> +		writel(0, master->regs + DEV_ID_RR1(slot));
> +		writel(0, master->regs + DEV_ID_RR2(slot));
> +	}
> +
> +	/*
> +	 * Enable Hot-Join and when a Hot-Join request happen, disable all

	                                               happens,

> +	 * events coming from this device.
> +	 *
> +	 * We will issue ENTDAA afterwards from the threaded IRQ handler.
> +	 */
> +	ctrl |= CTRL_HJ_ACK | CTRL_HJ_DISEC | CTRL_HALT_EN;
> +	writel(ctrl, master->regs + CTRL);
> +
> +	cdns_i3c_master_enable(master);
> +
> +	/*
> +	 * Reset all dynamic addresses on the bus, because we don't know what
> +	 * happened before this point (the bootloader may have assigned dynamic
> +	 * addresses that we're not aware of).
> +	 */
> +	ret = i3c_master_rstdaa_locked(m, I3C_BROADCAST_ADDR);
> +	if (ret)
> +		goto err_disable_master;
> +
> +	/* Disable all slave events (interrupts) before starting DAA. */
> +	events.events = I3C_CCC_EVENT_SIR | I3C_CCC_EVENT_MR |
> +			I3C_CCC_EVENT_HJ;
> +	ret = i3c_master_disec_locked(m, I3C_BROADCAST_ADDR, &events);
> +	if (ret)
> +		goto err_disable_master;
> +
> +	ret = cdns_i3c_master_do_daa_locked(master);
> +	if (ret < 0)
> +		goto err_disable_master;
> +
> +	/* Unmask Hot-Join and Marstership request interrupts. */

	Is that                Mastership ?

> +	events.events = I3C_CCC_EVENT_HJ | I3C_CCC_EVENT_MR;
> +	ret = i3c_master_enec_locked(m, I3C_BROADCAST_ADDR, &events);
> +	if (ret)
> +		pr_info("Failed to re-enable H-J");

		Not very good info...

> +
> +	writel(MST_INT_HJ_REQ, master->regs + MST_IER);
> +	return 0;
> +
> +err_disable_master:
> +	cdns_i3c_master_disable(master);
> +
> +err_detach_devs:
> +	cdns_i3c_master_bus_cleanup(m);
> +
> +	return ret;
> +}
> +

[snip]


-- 
~Randy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: DT dtc warnings
From: Alexandre Belloni @ 2017-12-14 19:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Barry Song, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
	Chen-Yu Tsai, Nicolas Ferre, Patrice Chotard, Peter Rosin,
	Santosh Shilimkar, arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKqiyCb8s9cyM-LdZv+hyMjv2ZpfGZmyy9_+Wrd7-qF8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 14/12/2017 at 13:00:07 -0600, Rob Herring wrote:
> On Thu, Dec 14, 2017 at 12:36 PM, Alexandre Belloni
> <alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Hi Rob,
> >
> > On 14/12/2017 at 12:21:06 -0600, Rob Herring wrote:
> >> Hi all,
> >>
> >> Below is a current list of ARM boards with more than 40 dtc warnings
> >> when building with W=1. There's a treewide patch in flight to fix some
> >> unit-address warnings[1], so those aren't included here. The list is
> >> grouped by maintainer. AT91, Exynos, and Allwinner continue to be at
> >> the top of the list and have been for some time (though having
> >> multiple boards for an SoC can cause lots of duplicated warnings).
> >>
> >> Please help fix these. More checks are being added[2].
> >>
> >
> > For the at91 ones, IIRC, they are coming from the clock binding and we
> > will have to break the ABI to fix it. This is not something we wanted to
> > do before 4.14 but it will happen at some point.
> 
> Looks like they are just missing unit-address. How does adding a
> unit-address break the ABI? Though, aren't you planning to change from
> a node per clock to clock controller node(s)? If so, then fixing when
> doing that is fine.

Adding the unit-address breaks the lookup of the clocks unless you want
to have nodes named prog0@0, prog1@1, pck0@8, etc...

I don't think it is worth the hassle going through all the dtsi to do
that.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] sfp: add sff module support
From: Florian Fainelli @ 2017-12-14 19:16 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Rob Herring; +Cc: devicetree, Mark Rutland, netdev
In-Reply-To: <E1ePQjz-0003q6-Tf@rmk-PC.armlinux.org.uk>


On 12/14/2017 02:27 AM, Russell King wrote:
> Add support for SFF modules, which are soldered down SFP modules.
> These have a different phys_id value, and also have the present and
> rate select signals omitted compared with their socketed counter-parts.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

Thanks!
-- 
Florian

^ permalink raw reply

* Re: [RFC PATCH 2/2] leds: as3645a: Update LED label generation
From: Dan Murphy @ 2017-12-14 19:13 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I,
	laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171214180335.j6qk2rrge77gpbba-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>

Sakari

On 12/14/2017 12:03 PM, Sakari Ailus wrote:
> Hi Dan,
> 
> Thanks for the patchset.
> 
> On Tue, Dec 12, 2017 at 03:50:24PM -0600, Dan Murphy wrote:
>> Generate the LED label based off either the
>> DT label node or off the I2C ID in the
>> i2c device id struct.
>>
>> If the label is used then this should denote
>> the LED function.  As an example it would
>> be as3645a:<function>
>>
>>  Otherwise if the label is not
>> used the LED label will be as3645a:flash and
>> as3645a:indicator.
> 
> Which tree the patch is for? I see "id" is a u32 in what I have, and
> also not defined in this scope.

Ah.  I did not notice that the driver used the probe_new call back.

I will be re-sending anyway since this is a RFC

Dan

> 
>>
>> Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
>> ---
>>  drivers/leds/leds-as3645a.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
>> index f883616d9e60..197acd69ddcc 100644
>> --- a/drivers/leds/leds-as3645a.c
>> +++ b/drivers/leds/leds-as3645a.c
>> @@ -526,10 +526,11 @@ static int as3645a_parse_node(struct as3645a *flash,
>>  
>>  	rval = of_property_read_string(flash->flash_node, "label", &name);
>>  	if (!rval)
>> -		strlcpy(names->flash, name, sizeof(names->flash));
>> +		snprintf(names->flash, sizeof(names->flash), "%s:%s",
>> +			 id->name, name);
>>  	else
>>  		snprintf(names->flash, sizeof(names->flash),
>> -			 "%s:flash", node->name);
>> +			 "%s:flash", id->name);
>>  
>>  	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
>>  				    &cfg->flash_timeout_us);
>> @@ -570,10 +571,11 @@ static int as3645a_parse_node(struct as3645a *flash,
>>  
>>  	rval = of_property_read_string(flash->indicator_node, "label", &name);
>>  	if (!rval)
>> -		strlcpy(names->indicator, name, sizeof(names->indicator));
>> +		snprintf(names->indicator, sizeof(names->indicator), "%s:%s",
>> +			 id->name, name);
>>  	else
>>  		snprintf(names->indicator, sizeof(names->indicator),
>> -			 "%s:indicator", node->name);
>> +			 "%s:indicator", id->name);
>>  
>>  	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
>>  				    &cfg->indicator_max_ua);
> 
> 
> 


-- 
------------------
Dan Murphy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: add sff,sff binding for SFP support
From: Florian Fainelli @ 2017-12-14 19:11 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Rob Herring
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <E1ePQju-0003ps-QW-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>



On 12/14/2017 02:27 AM, Russell King wrote:
> Add "sff,sff" for SFF module support with SFP.  These have a different
> phys_id value, and also have the present and rate select signals omitted
> compared with their socketed counter-parts.
> 
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>

Reviewed-by: Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: add sff,sff binding for SFP support
From: Rob Herring @ 2017-12-14 19:07 UTC (permalink / raw)
  To: Russell King
  Cc: Andrew Lunn, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Rutland,
	netdev
In-Reply-To: <E1ePQju-0003ps-QW-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>

On Thu, Dec 14, 2017 at 4:27 AM, Russell King
<rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:
> Add "sff,sff" for SFF module support with SFP.  These have a different
> phys_id value, and also have the present and rate select signals omitted
> compared with their socketed counter-parts.
>
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/net/sff,sfp.txt | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] arc: dts: Remove leading 0x and 0s from bindings notation
From: Rob Herring @ 2017-12-14 19:04 UTC (permalink / raw)
  To: Mathieu Malaterre
  Cc: Mark Rutland, Vineet Gupta, Alexey Brodkin,
	devicetree@vger.kernel.org, arcml, linux-kernel@vger.kernel.org
In-Reply-To: <20171214165344.27795-1-malat@debian.org>

On Thu, Dec 14, 2017 at 10:53 AM, Mathieu Malaterre <malat@debian.org> wrote:
> Improve the DTS files by removing all the leading "0x" and zeros to fix the
> following dtc warnings:
>
> Warning (unit_address_format): Node /XXX unit name should not have leading "0x"
>
> and
>
> Warning (unit_address_format): Node /XXX unit name should not have leading 0s
>
> Converted using the following command:
>
> find . -type f \( -iname *.dts -o -iname *.dtsi \) -exec sed -E -i -e "s/@0x([0-9a-fA-F\.]+)\s?\{/@\L\1 \{/g" -e "s/@0+([0-9a-fA-F\.]+)\s?\{/@\L\1 \{/g" {} +
>
> For simplicity, two sed expressions were used to solve each warnings separately.
>
> To make the regex expression more robust a few other issues were resolved,
> namely setting unit-address to lower case, and adding a whitespace before the
> the opening curly brace:
>
> https://elinux.org/Device_Tree_Linux#Linux_conventions
>
> This is a follow up to commit 4c9847b7375a ("dt-bindings: Remove leading 0x from bindings notation")
>
> Reported-by: David Daney <ddaney@caviumnetworks.com>
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---

For the series,

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: DT dtc warnings
From: Rob Herring @ 2017-12-14 19:00 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Barry Song, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
	Chen-Yu Tsai, Nicolas Ferre, Patrice Chotard, Peter Rosin,
	Santosh Shilimkar, arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20171214183642.GF2559-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org>

On Thu, Dec 14, 2017 at 12:36 PM, Alexandre Belloni
<alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> Hi Rob,
>
> On 14/12/2017 at 12:21:06 -0600, Rob Herring wrote:
>> Hi all,
>>
>> Below is a current list of ARM boards with more than 40 dtc warnings
>> when building with W=1. There's a treewide patch in flight to fix some
>> unit-address warnings[1], so those aren't included here. The list is
>> grouped by maintainer. AT91, Exynos, and Allwinner continue to be at
>> the top of the list and have been for some time (though having
>> multiple boards for an SoC can cause lots of duplicated warnings).
>>
>> Please help fix these. More checks are being added[2].
>>
>
> For the at91 ones, IIRC, they are coming from the clock binding and we
> will have to break the ABI to fix it. This is not something we wanted to
> do before 4.14 but it will happen at some point.

Looks like they are just missing unit-address. How does adding a
unit-address break the ABI? Though, aren't you planning to change from
a node per clock to clock controller node(s)? If so, then fixing when
doing that is fine.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 2/2] misc: Add Xilinx ZYNQMP VCU logicoreIP init driver
From: Randy Dunlap @ 2017-12-14 18:42 UTC (permalink / raw)
  To: Dhaval Shah, arnd-r2nGTMty4D4,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, pombredanne-od1rfyK75/E,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	michal.simek-gjFFaj9aHVfQT0dZR+AlfA, hyunk-gjFFaj9aHVfQT0dZR+AlfA,
	Dhaval Shah
In-Reply-To: <1513230920-9005-3-git-send-email-dshah-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>

On 12/13/2017 09:55 PM, Dhaval Shah wrote:
> Xilinx ZYNQMP logicoreIP Init driver is based on the new
> LogiCoreIP design created. This driver provides the processing system
> and programmable logic isolation. Set the frequency based on the clock
> information get from the logicoreIP register set.
> 
> It is put in drivers/misc as there is no subsystem for this logicoreIP.
> 
> Signed-off-by: Dhaval Shah <dshah-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes since v3:
>  No Changes.
> Changes since v2:
>  * Removed the "default n" from the Kconfig
>  * More help text added to explain more about the logicoreIP driver
>  * SPDX id is relocated at top of the file with // style comment
>  * Removed the export API and header file and make it a single driver
>    which provides logocoreIP init.
>  * Provide the information in commit message as well for the why driver
>    in drivers/misc.
> 
>  drivers/misc/Kconfig    |  15 ++
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/xlnx_vcu.c | 629 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 645 insertions(+)
>  create mode 100644 drivers/misc/xlnx_vcu.c
> 
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index f1a5c23..24ea516 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -496,6 +496,21 @@ config PCI_ENDPOINT_TEST
>             Enable this configuration option to enable the host side test driver
>             for PCI Endpoint.
>  
> +config XILINX_VCU
> +       tristate "Xilinx VCU logicoreIP Init"
> +       help

Please indent the help text (below) by 2 additional spaces, as documented
in coding-style.rst.

> +	  Provides the driver to enable and disable the isolation between the
> +	  processing system and programmable logic part by using the logicoreIP
> +	  register set. This driver also configure the frequency based on the

	                                 configures

> +	  clock information get from the logicoreIP register set.

                    drop   ^get^

> +
> +	  If you say yes here you get support for the logcoreIP.

	                                              logicoreIP.

> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called xlnx_vcu.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 5ca5f64..a6bd0b1 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE)		+= cxl/
>  obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
>  obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
>  obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
> +obj-$(CONFIG_XILINX_VCU)	+= xlnx_vcu.o
>  
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> diff --git a/drivers/misc/xlnx_vcu.c b/drivers/misc/xlnx_vcu.c
> new file mode 100644
> index 0000000..41819f0
> --- /dev/null
> +++ b/drivers/misc/xlnx_vcu.c
> @@ -0,0 +1,629 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx VCU Init
> + *
> + * Copyright (C) 2016 - 2017 Xilinx, Inc.
> + *
> + * Contacts   Dhaval Shah <dshah-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> + */
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +> +/* Address map for different registers implemented in the VCU LogiCORE IP. */

[snip]


> +/**
> + * xvcu_read - Read from the VCU register space
> + * @iomem:	vcu reg space base address
> + * @offset:	vcu reg offset from base
> + *
> + * Return:	Returns 32bit value from VCU register specified
> + *
> + */
> +static u32 xvcu_read(void __iomem *iomem, u32 offset)

You could inline the read() and write() functions...

> +{
> +	return ioread32(iomem + offset);
> +}
> +
> +/**
> + * xvcu_write - Write to the VCU register space
> + * @iomem:	vcu reg space base address
> + * @offset:	vcu reg offset from base
> + * @value:	Value to write
> + */
> +static void xvcu_write(void __iomem *iomem, u32 offset, u32 value)
> +{
> +	iowrite32(value, iomem + offset);
> +}
> +
> +/**
> + * xvcu_write_field_reg - Write to the vcu reg field
> + * @iomem:	vcu reg space base address
> + * @offset:	vcu reg offset from base
> + * @field:	vcu reg field to write to
> + * @mask:	vcu reg mask
> + * @shift:	vcu reg number of bits to shift the bitfield
> + */
> +static void xvcu_write_field_reg(void __iomem *iomem, int offset,
> +				u32 field, u32 mask, int shift)
> +{
> +	u32 val = xvcu_read(iomem, offset);
> +
> +	val &= ~(mask << shift);
> +	val |= (field & mask) << shift;
> +
> +	xvcu_write(iomem, offset, val);
> +}
> +
> +/**
> + * xvcu_set_vcu_pll_info - Set the VCU PLL info
> + * @xvcu:	Pointer to the xvcu_device structure
> + *
> + * Programming the VCU PLL based on the user configuration
> + * (ref clock freq, core clock freq, mcu clock freq).
> + * Core clock frequency has higher priority than mcu clock frequency
> + * Errors in following cases
> + *    - When mcu or clock clock get from logicoreIP is 0
> + *    - When VCU PLL DIV related bits value other than 1
> + *    - When proper data not found for given data
> + *    - When sis570_1 clocksource related operation failed
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +static int xvcu_set_vcu_pll_info(struct xvcu_device *xvcu)
> +{
> +	u32 refclk, coreclk, mcuclk, inte, deci;
> +	u32 divisor_mcu, divisor_core, fvco;
> +	u32 clkoutdiv, vcu_pll_ctrl, pll_clk;
> +	u32 cfg_val, mod, ctrl;
> +	int ret;
> +	unsigned int i;
> +	const struct xvcu_pll_cfg *found = NULL;
> +
> +	inte = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK);
> +	deci = xvcu_read(xvcu->logicore_reg_ba, VCU_PLL_CLK_DEC);
> +	coreclk = xvcu_read(xvcu->logicore_reg_ba, VCU_CORE_CLK) * MHZ;
> +	mcuclk = xvcu_read(xvcu->logicore_reg_ba, VCU_MCU_CLK) * MHZ;
> +	if (!mcuclk || !coreclk) {
> +		dev_err(xvcu->dev, "Invalid mcu and core clock data\n");
> +		return -EINVAL;
> +	}
> +
> +	refclk = (inte * MHZ) + (deci * (MHZ / FRAC));
> +	dev_dbg(xvcu->dev, "Ref clock from logicoreIP is %uHz\n", refclk);
> +	dev_dbg(xvcu->dev, "Core clock from logicoreIP is %uHz\n", coreclk);
> +	dev_dbg(xvcu->dev, "Mcu clock from logicoreIP is %uHz\n", mcuclk);
> +
> +	clk_disable_unprepare(xvcu->pll_ref);
> +	ret = clk_set_rate(xvcu->pll_ref, refclk);
> +	if (ret)
> +		dev_warn(xvcu->dev, "failed to set logicoreIP refclk rate\n");
> +
> +	ret = clk_prepare_enable(xvcu->pll_ref);
> +	if (ret) {
> +		dev_err(xvcu->dev, "failed to enable pll_ref clock source\n");
> +		return ret;
> +	}
> +
> +	refclk = clk_get_rate(xvcu->pll_ref);
> +
> +	/* The divide-by-2 should be always enabled (==1)

multi-line comment style:
	/*
	 * The divide-by-2 should always be enabled (==1)

> +	 * to meet the timing in the design.
> +	 * Otherwise, it's an error
> +	 */
> +	vcu_pll_ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_CTRL);
> +	clkoutdiv = vcu_pll_ctrl >> VCU_PLL_CTRL_CLKOUTDIV_SHIFT;
> +	clkoutdiv = clkoutdiv && VCU_PLL_CTRL_CLKOUTDIV_MASK;
> +	if (clkoutdiv != 1) {
> +		dev_err(xvcu->dev, "clkoutdiv value is invalid\n");
> +		return -EINVAL;
> +	}
> +
> +	for (i = ARRAY_SIZE(xvcu_pll_cfg) - 1; i > 0; i--) {
> +		const struct xvcu_pll_cfg *cfg = &xvcu_pll_cfg[i];
> +
> +		fvco = cfg->fbdiv * refclk;
> +		if (fvco >= FVCO_MIN && fvco <= FVCO_MAX) {
> +			pll_clk = fvco / VCU_PLL_DIV2;
> +			if (fvco % VCU_PLL_DIV2 != 0)
> +				pll_clk++;
> +			mod = pll_clk % coreclk;
> +			if (mod < LIMIT) {
> +				divisor_core = pll_clk / coreclk;
> +			} else if (coreclk - mod < LIMIT) {
> +				divisor_core = pll_clk / coreclk;
> +				divisor_core++;
> +			} else {
> +				continue;
> +			}
> +			if (divisor_core >= DIVISOR_MIN &&
> +			    divisor_core <= DIVISOR_MAX) {
> +				found = cfg;
> +				divisor_mcu = pll_clk / mcuclk;
> +				mod = pll_clk % mcuclk;
> +				if (mcuclk - mod < LIMIT)
> +					divisor_mcu++;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (!found) {
> +		dev_err(xvcu->dev, "Invalid clock combination.\n");
> +		return -EINVAL;
> +	}
> +
> +	xvcu->coreclk = pll_clk / divisor_core;
> +	mcuclk = pll_clk / divisor_mcu;
> +	dev_dbg(xvcu->dev, "Actual Ref clock freq is %uHz\n", refclk);
> +	dev_dbg(xvcu->dev, "Actual Core clock freq is %uHz\n", xvcu->coreclk);
> +	dev_dbg(xvcu->dev, "Actual Mcu clock freq is %uHz\n", mcuclk);
> +
> +	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_FBDIV_MASK << VCU_PLL_CTRL_FBDIV_SHIFT);
> +	vcu_pll_ctrl |= (found->fbdiv & VCU_PLL_CTRL_FBDIV_MASK) <<
> +			 VCU_PLL_CTRL_FBDIV_SHIFT;
> +	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_POR_IN_MASK <<
> +			  VCU_PLL_CTRL_POR_IN_SHIFT);
> +	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_POR_IN_MASK) <<
> +			 VCU_PLL_CTRL_POR_IN_SHIFT;
> +	vcu_pll_ctrl &= ~(VCU_PLL_CTRL_PWR_POR_MASK <<
> +			  VCU_PLL_CTRL_PWR_POR_SHIFT);
> +	vcu_pll_ctrl |= (VCU_PLL_CTRL_DEFAULT & VCU_PLL_CTRL_PWR_POR_MASK) <<
> +			 VCU_PLL_CTRL_PWR_POR_SHIFT;
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CTRL, vcu_pll_ctrl);
> +
> +	/* Set divisor for the core and mcu clock */
> +	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL);
> +	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> +	ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> +		 VCU_PLL_DIVISOR_SHIFT;
> +	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> +	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_CORE_CTRL, ctrl);
> +
> +	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL);
> +	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> +	ctrl |= (divisor_core & VCU_PLL_DIVISOR_MASK) <<
> +		 VCU_PLL_DIVISOR_SHIFT;
> +	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> +	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_CORE_CTRL, ctrl);
> +
> +	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL);
> +	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> +	ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
> +	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> +	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_ENC_MCU_CTRL, ctrl);
> +
> +	ctrl = xvcu_read(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL);
> +	ctrl &= ~(VCU_PLL_DIVISOR_MASK << VCU_PLL_DIVISOR_SHIFT);
> +	ctrl |= (divisor_mcu & VCU_PLL_DIVISOR_MASK) << VCU_PLL_DIVISOR_SHIFT;
> +	ctrl &= ~(VCU_SRCSEL_MASK << VCU_SRCSEL_SHIFT);
> +	ctrl |= (VCU_SRCSEL_PLL & VCU_SRCSEL_MASK) << VCU_SRCSEL_SHIFT;
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_DEC_MCU_CTRL, ctrl);
> +
> +	/* Set RES, CP, LFHF, LOCK_CNT and LOCK_DLY cfg values */
> +	cfg_val = (found->res << VCU_PLL_CFG_RES_SHIFT) |
> +		   (found->cp << VCU_PLL_CFG_CP_SHIFT) |
> +		   (found->lfhf << VCU_PLL_CFG_LFHF_SHIFT) |
> +		   (found->lock_cnt << VCU_PLL_CFG_LOCK_CNT_SHIFT) |
> +		   (found->lock_dly << VCU_PLL_CFG_LOCK_DLY_SHIFT);
> +	xvcu_write(xvcu->vcu_slcr_ba, VCU_PLL_CFG, cfg_val);
> +
> +	return 0;
> +}
> +
> +/**
> + * xvcu_set_pll - PLL init sequence
> + * @xvcu:	Pointer to the xvcu_device structure
> + *
> + * Call the api to set the PLL info and once that is done then
> + * init the PLL sequence to make the PLL stable.
> + *
> + * Return:	Returns status, either success or error+reason
> + */
> +static int xvcu_set_pll(struct xvcu_device *xvcu)
> +{
> +	u32 lock_status;
> +	unsigned long timeout;
> +	int ret;
> +
> +	ret = xvcu_set_vcu_pll_info(xvcu);
> +	if (ret) {
> +		dev_err(xvcu->dev, "failed to set pll info\n");
> +		return ret;
> +	}
> +
> +	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> +			     1, VCU_PLL_CTRL_BYPASS_MASK,
> +			     VCU_PLL_CTRL_BYPASS_SHIFT);
> +	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> +			     1, VCU_PLL_CTRL_RESET_MASK,
> +			     VCU_PLL_CTRL_RESET_SHIFT);
> +	xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> +			     0, VCU_PLL_CTRL_RESET_MASK,
> +			     VCU_PLL_CTRL_RESET_SHIFT);
> +	/* Defined the timeout for the max time to wait the

comment style.

> +	 * PLL_STATUS to be locked.
> +	 */
> +	timeout = jiffies + msecs_to_jiffies(2000);
> +	do {
> +		lock_status = xvcu_read(xvcu->vcu_slcr_ba, VCU_PLL_STATUS);
> +		if (lock_status & VCU_PLL_STATUS_LOCK_STATUS_MASK) {
> +			xvcu_write_field_reg(xvcu->vcu_slcr_ba, VCU_PLL_CTRL,
> +					     0, VCU_PLL_CTRL_BYPASS_MASK,
> +					     VCU_PLL_CTRL_BYPASS_SHIFT);
> +			return 0;
> +		}
> +	} while (!time_after(jiffies, timeout));
> +
> +	/* PLL is not locked even after the timeout of the 2sec */
> +	dev_err(xvcu->dev, "PLL is not locked\n");
> +	return -ETIMEDOUT;
> +}
> +
> +/**
> + * xvcu_probe - Probe existence of the logicoreIP
> + *			and initialize PLL
> + *
> + * @pdev:	Pointer to the platform_device structure
> + *
> + * Return:	Returns 0 on success
> + *		Negative error code otherwise
> + */
> +static int xvcu_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct xvcu_device *xvcu;
> +	int ret;
> +
> +	xvcu = devm_kzalloc(&pdev->dev, sizeof(*xvcu), GFP_KERNEL);
> +	if (!xvcu)
> +		return -ENOMEM;
> +
> +	xvcu->dev = &pdev->dev;
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "vcu_slcr");
> +	if (!res) {
> +		dev_err(&pdev->dev, "get vcu_slcr memory resource failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	xvcu->vcu_slcr_ba = devm_ioremap_nocache(&pdev->dev,
> +			res->start, resource_size(res));
> +	if (!xvcu->vcu_slcr_ba) {
> +		dev_err(&pdev->dev, "vcu_slcr register mapping failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "logicore");
> +	if (!res) {
> +		dev_err(&pdev->dev, "get logicore memory resource failed.\n");
> +		return -ENODEV;
> +	}
> +
> +	xvcu->logicore_reg_ba = devm_ioremap_nocache(&pdev->dev,
> +			res->start, resource_size(res));
> +	if (!xvcu->logicore_reg_ba) {
> +		dev_err(&pdev->dev, "logicore register mapping failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	xvcu->aclk = devm_clk_get(&pdev->dev, "aclk");
> +	if (IS_ERR(xvcu->aclk)) {
> +		dev_err(&pdev->dev, "Could not get aclk clock\n");
> +		return PTR_ERR(xvcu->aclk);
> +	}
> +
> +	xvcu->pll_ref = devm_clk_get(&pdev->dev, "pll_ref");
> +	if (IS_ERR(xvcu->pll_ref)) {
> +		dev_err(&pdev->dev, "Could not get pll_ref clock\n");
> +		return PTR_ERR(xvcu->pll_ref);
> +	}
> +
> +	ret = clk_prepare_enable(xvcu->aclk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "aclk clock enable failed\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(xvcu->pll_ref);
> +	if (ret) {
> +		dev_err(&pdev->dev, "pll_ref clock enable failed\n");
> +		goto error_aclk;
> +	}
> +
> +	/* Do the Gasket isolation and put the VCU out of reset

comment style.

> +	 * Bit 0 : Gasket isolation
> +	 * Bit 1 : put VCU out of reset
> +	 */
> +	xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, VCU_GASKET_VALUE);
> +
> +	/* Do the PLL Settings based on the ref clk,core and mcu clk freq */
> +	ret = xvcu_set_pll(xvcu);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to set the pll\n");
> +		goto error_pll_ref;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, xvcu);
> +
> +	dev_info(&pdev->dev, "%s: Probed successfully\n", __func__);
> +
> +	return 0;
> +
> +error_pll_ref:
> +	clk_disable_unprepare(xvcu->pll_ref);
> +error_aclk:
> +	clk_disable_unprepare(xvcu->aclk);
> +	return ret;
> +}
> +
> +/**
> + * xvcu_remove - Insert gasket isolation
> + *			and disable the clock
> + * @pdev:	Pointer to the platform_device structure
> + *
> + * Return:	Returns 0 on success
> + *		Negative error code otherwise
> + */
> +static int xvcu_remove(struct platform_device *pdev)
> +{
> +	struct xvcu_device *xvcu;
> +
> +	xvcu = platform_get_drvdata(pdev);
> +	if (!xvcu)
> +		return -ENODEV;
> +
> +	/* Add the the Gasket isolation and put the VCU in reset.

style.

> +	 */
> +	xvcu_write(xvcu->logicore_reg_ba, VCU_GASKET_INIT, 0);
> +
> +	clk_disable_unprepare(xvcu->pll_ref);
> +	clk_disable_unprepare(xvcu->aclk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id xvcu_of_id_table[] = {
> +	{ .compatible = "xlnx,vcu" },
> +	{ .compatible = "xlnx,vcu-logicoreip-1.0" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, xvcu_of_id_table);
> +
> +static struct platform_driver xvcu_driver = {
> +	.driver = {
> +		.name           = "xilinx-vcu",
> +		.of_match_table = xvcu_of_id_table,
> +	},
> +	.probe                  = xvcu_probe,
> +	.remove                 = xvcu_remove,
> +};
> +
> +module_platform_driver(xvcu_driver);
> +
> +MODULE_AUTHOR("Dhaval Shah <dshah-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>");
> +MODULE_DESCRIPTION("Xilinx VCU init Driver");
> +MODULE_LICENSE("GPL v2");
> 

The code has several divide operations. Does it build OK for 32-bit target?

-- 
~Randy
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] arm64: dts: Remove leading 0x and 0s from bindings notation
From: Rob Herring @ 2017-12-14 18:42 UTC (permalink / raw)
  To: Joe Perches
  Cc: Matthias Brugger, Mathieu Malaterre, Wei Xu, Mark Rutland,
	Catalin Marinas, Will Deacon, Andy Gross, David Brown, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-arm-msm,
	ARM/QUALCOMM 
In-Reply-To: <1513276199.27409.75.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>

On Thu, Dec 14, 2017 at 12:29 PM, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> On Thu, 2017-12-14 at 19:03 +0100, Matthias Brugger wrote:
>>
>> On 12/14/2017 05:53 PM, Mathieu Malaterre wrote:
>> [...]
>> > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > index 26396ef53bde..0446b122a6e2 100644
>> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> > @@ -249,7 +249,7 @@
>> >                     reg = <0 0x10005000 0 0x1000>;
>> >             };
>> >
>> > -           pio: pinctrl@0x10005000 {
>> > +           pio: pinctrl@10005000 {
>> >                     compatible = "mediatek,mt8173-pinctrl";
>> >                     reg = <0 0x1000b000 0 0x1000>;
>> >                     mediatek,pctl-regmap = <&syscfg_pctl_a>;
>>
>> Acked-by: Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Should all of these be fixed?
>
> $ git grep -P "^\s*\w+:\s*[\w\-]+@0[xX]" -- "*.dts*" | wc -l
> 69

Yes, there's patches for all arches.

>
> Is this a pattern that should be added to checkpatch?

No, because dtc provides the warnings.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: DT dtc warnings
From: Alexandre Belloni @ 2017-12-14 18:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: Barry Song, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
	Chen-Yu Tsai, Nicolas Ferre, Patrice Chotard, Peter Rosin,
	Santosh Shilimkar, arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqJKCzjx3zNigu_0T=Ku=_P2SQEjTx=uVbvr_QxJQR7kYg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Rob,

On 14/12/2017 at 12:21:06 -0600, Rob Herring wrote:
> Hi all,
> 
> Below is a current list of ARM boards with more than 40 dtc warnings
> when building with W=1. There's a treewide patch in flight to fix some
> unit-address warnings[1], so those aren't included here. The list is
> grouped by maintainer. AT91, Exynos, and Allwinner continue to be at
> the top of the list and have been for some time (though having
> multiple boards for an SoC can cause lots of duplicated warnings).
> 
> Please help fix these. More checks are being added[2].
> 

For the at91 ones, IIRC, they are coming from the clock binding and we
will have to break the ABI to fix it. This is not something we wanted to
do before 4.14 but it will happen at some point.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 00/12] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator
From: Philippe Ombredanne @ 2017-12-14 18:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Bird, Timothy, Takiguchi, Yasunari, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-media@vger.kernel.org, tbird20d@gmail.com,
	frowand.list@gmail.com, Yamamoto, Masayuki, Nozawa, Hideki (STWN),
	Yonezawa, Kota, Matsumoto, Toshihiko, Watanabe, Satoshi (SSS)
In-Reply-To: <20171214160411.32f23456@vento.lan>

On Thu, Dec 14, 2017 at 7:04 PM, Mauro Carvalho Chehab
<mchehab@s-opensource.com> wrote:
> Em Thu, 14 Dec 2017 17:32:34 +0000
> "Bird, Timothy" <Tim.Bird@sony.com> escreveu:
>
>> > -----Original Message-----
>> > From: Philippe on Thursday, December 14, 2017 6:25 AM
>> > Dear Mauro,
>> >
>> > On Thu, Dec 14, 2017 at 11:55 AM, Mauro Carvalho Chehab
>> > <mchehab@s-opensource.com> wrote:
>> >
>> > > SPDX is a new requirement that started late on Kernel 4.14 development
>> > > cycle (and whose initial changes were merged directly at Linus tree).
>> > > Not all existing files have it yet, as identifying the right license
>> > > on existing files is a complex task, but if you do a:
>> > >
>> > >         $ git grep SPDX $(find . -name Makefile) $(find . -name Kconfig)
>> > >
>> > > You'll see that lot of such files have it already.
>> >
>> > FWIW, short of having SPDX tags, identifying the right license on
>> > existing files is not a super complex task: if boils down to running
>> > many diffs.
>> >
>> > Take the ~60K files in kernel, and about 6K license and notices
>> > reference texts. Then compute a pairwise diff of each of the 60K file
>> > against the 6K reference texts. Repeat the pairwise diff a few more
>> > times, say 10 times, as multiple licenses may appear in any given
>> > kernel file. And keep the diffs that have the fewest
>> > difference/highest similarity with the reference texts as the detected
>> > license. Done!
>>
>> You can't do license detection and assignment in this automated fashion -
>> at least not generally.
>>
>> Even a single word of difference between the notice in the source
>> code and the reference license notice or text may have legal implications
>> that are not conveyed by the simplified SPDX tag.  When differences are
>> found, we're going to have to kick the discrepancies to a human for review.
>> This is especially true for files with multiple licenses.
>>
>> For a work of original authorship, or a single copyright holder, the author
>> or copyright holder may be able to change the notice or text, or gloss
>> over any difference from the reference text, and make the SPDX  assignment
>> (or even change the license, if they want).  This would apply to something
>> new like this Sony driver.  However, for code that is already in the kernel
>> tree, with likely multiple contributors, the legal situation gets a little
>> more murky.
>
> Precisely. This is easily fixable when the code author changes the
> license text, or when someone from the Company that holds copyrights sends
> the SPDX markups (as emails @company can be seen as official e-mails, except
> if explicitly noticed otherwise). So, from my side, I'm now requiring
> SPDX for new drivers.
>
> However, if someone else is doing the changes, it can be tricky and risky
> to pick up the patch, adding my SOB, if not endorsed by the copyright
> owners, or by LF legal counseling. So, I prefer to not pick those myself,
> except from people I trust.

Exactly, and this why --after the first batch that Greg pushed and
Linus pulled and that had been carefully reviewed--, I am trying to
gently nit the submitters of new patches, one at a time to use the new
SPDX tags. Eventually if I can find the time, I could also submit some
bigger patches to add SPDX tags to a bunch of files at once but that
would have to be organized in small batches by copyright holder and
these would be only RFCs until reviewed by and agreed to by the actual
copyright holders.

-- 
Cordially
Philippe Ombredanne

^ permalink raw reply

* Re: [PATCH] arm64: dts: Remove leading 0x and 0s from bindings notation
From: Joe Perches @ 2017-12-14 18:29 UTC (permalink / raw)
  To: Matthias Brugger, Mathieu Malaterre, Rob Herring
  Cc: Wei Xu, Mark Rutland, Catalin Marinas, Will Deacon, Andy Gross,
	David Brown, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	linux-arm-kernel, devicetree, linux-kernel, linux-mediatek,
	linux-arm-msm, linux-soc
In-Reply-To: <98a7dc30-e736-eaf8-204f-2ac2b3ea79b9@gmail.com>

On Thu, 2017-12-14 at 19:03 +0100, Matthias Brugger wrote:
> 
> On 12/14/2017 05:53 PM, Mathieu Malaterre wrote:
> [...]
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > index 26396ef53bde..0446b122a6e2 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> > @@ -249,7 +249,7 @@
> >  			reg = <0 0x10005000 0 0x1000>;
> >  		};
> >  
> > -		pio: pinctrl@0x10005000 {
> > +		pio: pinctrl@10005000 {
> >  			compatible = "mediatek,mt8173-pinctrl";
> >  			reg = <0 0x1000b000 0 0x1000>;
> >  			mediatek,pctl-regmap = <&syscfg_pctl_a>;
> 
> Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

Should all of these be fixed?

$ git grep -P "^\s*\w+:\s*[\w\-]+@0[xX]" -- "*.dts*" | wc -l
69

Is this a pattern that should be added to checkpatch?

^ permalink raw reply

* Re: [PATCH v4 00/12] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator
From: Philippe Ombredanne @ 2017-12-14 18:22 UTC (permalink / raw)
  To: Bird, Timothy
  Cc: Mauro Carvalho Chehab, Takiguchi, Yasunari,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-media@vger.kernel.org,
	tbird20d@gmail.com, frowand.list@gmail.com, Yamamoto, Masayuki,
	Nozawa, Hideki (STWN), Yonezawa, Kota, Matsumoto, Toshihiko,
	Watanabe, Satoshi (SSS)
In-Reply-To: <ECADFF3FD767C149AD96A924E7EA6EAF40AE4BB5@USCULXMSG01.am.sony.com>

Tim,

On Thu, Dec 14, 2017 at 6:32 PM, Bird, Timothy <Tim.Bird@sony.com> wrote:
>
>
>> -----Original Message-----
>> From: Philippe on Thursday, December 14, 2017 6:25 AM
>> Dear Mauro,
>>
>> On Thu, Dec 14, 2017 at 11:55 AM, Mauro Carvalho Chehab
>> <mchehab@s-opensource.com> wrote:
>>
>> > SPDX is a new requirement that started late on Kernel 4.14 development
>> > cycle (and whose initial changes were merged directly at Linus tree).
>> > Not all existing files have it yet, as identifying the right license
>> > on existing files is a complex task, but if you do a:
>> >
>> >         $ git grep SPDX $(find . -name Makefile) $(find . -name Kconfig)
>> >
>> > You'll see that lot of such files have it already.
>>
>> FWIW, short of having SPDX tags, identifying the right license on
>> existing files is not a super complex task: if boils down to running
>> many diffs.
>>
>> Take the ~60K files in kernel, and about 6K license and notices
>> reference texts. Then compute a pairwise diff of each of the 60K file
>> against the 6K reference texts. Repeat the pairwise diff a few more
>> times, say 10 times, as multiple licenses may appear in any given
>> kernel file. And keep the diffs that have the fewest
>> difference/highest similarity with the reference texts as the detected
>> license. Done!
>
> You can't do license detection and assignment in this automated fashion -
> at least not generally.
>
> Even a single word of difference between the notice in the source
> code and the reference license notice or text may have legal implications
> that are not conveyed by the simplified SPDX tag.  When differences are
> found, we're going to have to kick the discrepancies to a human for review.
> This is especially true for files with multiple licenses.
>
> For a work of original authorship, or a single copyright holder, the author
> or copyright holder may be able to change the notice or text, or gloss
> over any difference from the reference text, and make the SPDX  assignment
> (or even change the license, if they want).  This would apply to something
> new like this Sony driver.  However, for code that is already in the kernel
> tree, with likely multiple contributors, the legal situation gets a little
> more murky.
>
> I suspect the vast majority of the ~60k files will probably fall neatly into an
> SPDX category, but I'm guessing a fair number (maybe hundreds) will require
> some review and discussion.

You are completely and 100% right. I was just describing the mechanics
of the license detection side. The actual process has been and always
will be scan then review carefully and then discuss. There is no sane
automated tool that could do it all for sure.

As a funny side note, there are over 80+ licenses in the kernel and
there is (or rather was before starting adding SPDX tags) 1000+
different license notices and over 700+ variations of "this file in
under the GPL"... This starts to diminish a bit with the addition of
SPDX tags and eventually most or all boilerplate could be removed over
time with reviews and discussions, IMHO for the better: I will then be
able to trash my tool and use a good ole grep instead ;)

-- 
Cordially
Philippe Ombredanne

^ permalink raw reply

* DT dtc warnings
From: Rob Herring @ 2017-12-14 18:21 UTC (permalink / raw)
  To: Barry Song, Kukjin Kim, Krzysztof Kozlowski, Maxime Ripard,
	Chen-Yu Tsai, Nicolas Ferre, Alexandre Belloni, Patrice Chotard,
	Peter Rosin, Santosh Shilimkar,
	arm-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Hi all,

Below is a current list of ARM boards with more than 40 dtc warnings
when building with W=1. There's a treewide patch in flight to fix some
unit-address warnings[1], so those aren't included here. The list is
grouped by maintainer. AT91, Exynos, and Allwinner continue to be at
the top of the list and have been for some time (though having
multiple boards for an SoC can cause lots of duplicated warnings).

Please help fix these. More checks are being added[2].

Rob

[1] https://patchwork.kernel.org/patch/10112725/
[2] https://www.spinics.net/lists/devicetree-compiler/msg01620.html


77 - arch/arm/boot/dts/prima2-evb.dts

50 - arch/arm/boot/dts/exynos5250-arndale.dts
50 - arch/arm/boot/dts/exynos5250-smdk5250.dts
50 - arch/arm/boot/dts/exynos5250-snow.dts
50 - arch/arm/boot/dts/exynos5250-snow-rev5.dts
50 - arch/arm/boot/dts/exynos5250-spring.dts
71 - arch/arm/boot/dts/exynos5420-arndale-octa.dts
71 - arch/arm/boot/dts/exynos5420-peach-pit.dts
71 - arch/arm/boot/dts/exynos5420-smdk5420.dts
71 - arch/arm/boot/dts/exynos5422-odroidhc1.dts
71 - arch/arm/boot/dts/exynos5422-odroidxu3.dts
71 - arch/arm/boot/dts/exynos5422-odroidxu3-lite.dts
71 - arch/arm/boot/dts/exynos5422-odroidxu4.dts
71 - arch/arm/boot/dts/exynos5800-peach-pi.dts

42 - arch/arm/boot/dts/sun5i-gr8-evb.dts
44 - arch/arm/boot/dts/sun5i-a10s-olinuxino-micro.dts
49 - arch/arm/boot/dts/sun7i-a20-icnova-swac.dts
50 - arch/arm/boot/dts/sun7i-a20-itead-ibox.dts
50 - arch/arm/boot/dts/sun7i-a20-m3.dts
51 - arch/arm/boot/dts/sun7i-a20-cubieboard2.dts
51 - arch/arm/boot/dts/sun7i-a20-mk808c.dts
51 - arch/arm/boot/dts/sun7i-a20-wits-pro-a20-dkt.dts
53 - arch/arm/boot/dts/sun7i-a20-bananapi.dts
53 - arch/arm/boot/dts/sun7i-a20-bananapi-m1-plus.dts
53 - arch/arm/boot/dts/sun7i-a20-hummingbird.dts
53 - arch/arm/boot/dts/sun7i-a20-i12-tvbox.dts
53 - arch/arm/boot/dts/sun7i-a20-lamobo-r1.dts
53 - arch/arm/boot/dts/sun7i-a20-olinuxino-lime.dts
53 - arch/arm/boot/dts/sun7i-a20-pcduino3-nano.dts
54 - arch/arm/boot/dts/sun7i-a20-olinuxino-lime2.dts
54 - arch/arm/boot/dts/sun7i-a20-olinuxino-lime2-emmc.dts
55 - arch/arm/boot/dts/sun7i-a20-bananapro.dts
55 - arch/arm/boot/dts/sun7i-a20-cubietruck.dts
55 - arch/arm/boot/dts/sun7i-a20-orangepi.dts
55 - arch/arm/boot/dts/sun7i-a20-pcduino3.dts
55 - arch/arm/boot/dts/sun7i-a20-wexler-tab7200.dts
56 - arch/arm/boot/dts/sun7i-a20-orangepi-mini.dts
61 - arch/arm/boot/dts/sun7i-a20-olimex-som-evb.dts
61 - arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts
61 - arch/arm/boot/dts/sun7i-a20-olinuxino-micro-emmc.dts

46 - arch/arm/boot/dts/at91sam9rlek.dts
48 - arch/arm/boot/dts/at91sam9261ek.dts
50 - arch/arm/boot/dts/at91-kizbox.dts
50 - arch/arm/boot/dts/at91-qil_a9260.dts
51 - arch/arm/boot/dts/at91rm9200ek.dts
52 - arch/arm/boot/dts/at91sam9g20ek.dts
52 - arch/arm/boot/dts/at91-sam9_l9260.dts
53 - arch/arm/boot/dts/at91-foxg20.dts
53 - arch/arm/boot/dts/at91sam9260ek.dts
53 - arch/arm/boot/dts/at91sam9g20ek_2mmc.dts
54 - arch/arm/boot/dts/at91sam9263ek.dts
56 - arch/arm/boot/dts/at91sam9n12ek.dts
61 - arch/arm/boot/dts/at91-cosino_mega2560.dts
61 - arch/arm/boot/dts/at91sam9m10g45ek.dts
62 - arch/arm/boot/dts/at91-ariettag25.dts
63 - arch/arm/boot/dts/at91-ariag25.dts
64 - arch/arm/boot/dts/at91-kizboxmini.dts
64 - arch/arm/boot/dts/at91sam9g15ek.dts
65 - arch/arm/boot/dts/at91sam9g25ek.dts
66 - arch/arm/boot/dts/at91sam9g35ek.dts
69 - arch/arm/boot/dts/at91sam9x25ek.dts
70 - arch/arm/boot/dts/at91sam9x35ek.dts
74 - arch/arm/boot/dts/sama5d33ek.dts
75 - arch/arm/boot/dts/at91-sama5d27_som1_ek.dts
75 - arch/arm/boot/dts/at91-sama5d2_xplained.dts
76 - arch/arm/boot/dts/at91-kizbox2.dts
77 - arch/arm/boot/dts/sama5d31ek.dts
80 - arch/arm/boot/dts/sama5d34ek.dts
81 - arch/arm/boot/dts/sama5d35ek.dts
84 - arch/arm/boot/dts/at91-sama5d3_xplained.dts
84 - arch/arm/boot/dts/sama5d36ek_cmp.dts
84 - arch/arm/boot/dts/sama5d36ek.dts
93 - arch/arm/boot/dts/at91-sama5d4ek.dts
93 - arch/arm/boot/dts/at91-sama5d4_xplained.dts
93 - arch/arm/boot/dts/at91-vinco.dts
94 - arch/arm/boot/dts/at91-sama5d4_ma5d4evk.dts
77 - arch/arm/boot/dts/at91-tse850-3.dts

40 - arch/arm/boot/dts/stih410-b2260.dts
43 - arch/arm/boot/dts/stih410-b2120.dts

52 - arch/arm/boot/dts/keystone-k2e-evm.dts
73 - arch/arm/boot/dts/keystone-k2l-evm.dts
97 - arch/arm/boot/dts/keystone-k2hk-evm.dts

Boards with no maintainer:
192 - arch/arm/boot/dts/atlas7-evb.dts
50 - arch/arm/boot/dts/aks-cdu.dts
50 - arch/arm/boot/dts/animeo_ip.dts
50 - arch/arm/boot/dts/ethernut5.dts
50 - arch/arm/boot/dts/evk-pro3.dts
50 - arch/arm/boot/dts/tny_a9260.dts
50 - arch/arm/boot/dts/tny_a9g20.dts
50 - arch/arm/boot/dts/usb_a9260.dts
50 - arch/arm/boot/dts/usb_a9g20.dts
50 - arch/arm/boot/dts/usb_a9g20_lpw.dts
51 - arch/arm/boot/dts/mpa1600.dts
54 - arch/arm/boot/dts/tny_a9263.dts
54 - arch/arm/boot/dts/usb_a9263.dts
60 - arch/arm/boot/dts/pm9g45.dts
75 - arch/arm/boot/dts/ste-nomadik-nhk15.dts
75 - arch/arm/boot/dts/ste-nomadik-s8815.dts
77 - arch/arm/boot/dts/atlas6-evb.dts
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [patch v14 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Philippe Ombredanne @ 2017-12-14 18:14 UTC (permalink / raw)
  To: Oleksandr Shamray
  Cc: Greg Kroah-Hartman, Arnd Bergmann, LKML,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	OpenBMC Maillist, joel-U3u1mxZcP9KHXe+LvDLADg,
	Jiří Pírko, Tobias Klauser,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	vadimp-VPRAkNaXOzVWk0Htik3J/w,
	system-sw-low-level-VPRAkNaXOzVWk0Htik3J/w, Rob Herring,
	openocd-devel-owner-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-api-u79uwXL29TY76Z2rM5mHXA, David S. Miller,
	Mauro Carvalho Chehab, Jiri Pirko
In-Reply-To: <1513268971-13518-3-git-send-email-oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Oleksandr,

On Thu, Dec 14, 2017 at 5:29 PM, Oleksandr Shamray
<oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> Driver adds support of Aspeed 2500/2400 series SOC JTAG master controller.
>
> Driver implements the following jtag ops:
> - freq_get;
> - freq_set;
> - status_get;
> - idle;
> - xfer;
>
> It has been tested on Mellanox system with BMC equipped with
> Aspeed 2520 SoC for programming CPLD devices.
>
> Signed-off-by: Oleksandr Shamray <oleksandrs-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jiri Pirko <jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
> v13->v14
> Comments pointed by Philippe Ombredanne <pombredanne-od1rfyK75/E@public.gmane.org>
> - Change style of head block comment from /**/ to //
>
> v12->v13
> Comments pointed by Philippe Ombredanne <pombredanne-od1rfyK75/E@public.gmane.org>
> - Change jtag-aspeed.c licence type to
>   SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>   and reorder line with license in description

Thanks for the SPDX tags! for this part you have my ack:

Acked-by: Philippe Ombredanne <pombredanne-kIH2VFuay/A@public.gmane.org?

--
Cordially
Philippe Ombredanne

^ permalink raw reply

* Re: [patch v14 1/4] drivers: jtag: Add JTAG core driver
From: Philippe Ombredanne @ 2017-12-14 18:13 UTC (permalink / raw)
  To: Oleksandr Shamray
  Cc: Greg Kroah-Hartman, Arnd Bergmann, LKML,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	OpenBMC Maillist, joel, Jiří Pírko, Tobias Klauser,
	linux-serial, vadimp, system-sw-low-level, Rob Herring,
	openocd-devel-owner, linux-api, David S. Miller,
	Mauro Carvalho Chehab, Jiri Pirko
In-Reply-To: <1513268971-13518-2-git-send-email-oleksandrs@mellanox.com>

Oleksandr,

On Thu, Dec 14, 2017 at 5:29 PM, Oleksandr Shamray
<oleksandrs@mellanox.com> wrote:
> Initial patch for JTAG driver
> JTAG class driver provide infrastructure to support hardware/software
> JTAG platform drivers. It provide user layer API interface for flashing
> and debugging external devices which equipped with JTAG interface
> using standard transactions.
>
> Driver exposes set of IOCTL to user space for:
> - XFER:
> - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);
> - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);
> - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified
>   number of clocks).
> - SIOCFREQ/GIOCFREQ for setting and reading JTAG frequency.
>
> Driver core provides set of internal APIs for allocation and
> registration:
> - jtag_register;
> - jtag_unregister;
> - jtag_alloc;
> - jtag_free;
>
> Platform driver on registration with jtag-core creates the next
> entry in dev folder:
> /dev/jtagX
>
> Signed-off-by: Oleksandr Shamray <oleksandrs@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v13->v14
> Comments pointed by Philippe Ombredanne <pombredanne@nexb.com>
> - Change style of head block comment from /**/ to //
>
> v12->v13
> Comments pointed by Philippe Ombredanne <pombredanne@nexb.com>
> - Change jtag.c licence type to
>   SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>   and reorder line with license in description
> v11->v12
> Comments pointed by Greg KH <gregkh@linuxfoundation.org>
> - Change jtag.h licence type to
>   SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
>   and reorder line with license in description

Thanks for the SPDX bits: for this part you have my ack:

Acked-by: Philippe Ombredanne <pombredanne@nexB.com?

-- 
Cordially
Philippe Ombredanne

^ permalink raw reply

* Re: [PATCH v2 1/2] leds: as3645a: Fix quoted string split warning
From: Sakari Ailus @ 2017-12-14 18:04 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel,
	laurent.pinchart, devicetree, linux-kernel, linux-leds
In-Reply-To: <20171214173727.10070-1-dmurphy@ti.com>

On Thu, Dec 14, 2017 at 11:37:26AM -0600, Dan Murphy wrote:
> Fix a warning for a split quoted string across
> lines.
> 
> WARNING: quoted string split across lines
> 459: FILE: drivers/leds/leds-as3645a.c:459:
> 		dev_err(dev, "AS3645A not detected "
> 			"(model %d rfu %d)\n", model, rfu);
> 
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Dan Murphy <dmurphy@ti.com>

Thanks!

For both:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

^ permalink raw reply

* Re: [PATCH v4 00/12] [dt-bindings] [media] Add document file and driver for Sony CXD2880 DVB-T2/T tuner + demodulator
From: Mauro Carvalho Chehab @ 2017-12-14 18:04 UTC (permalink / raw)
  To: Bird, Timothy
  Cc: Philippe Ombredanne, Takiguchi, Yasunari,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-media@vger.kernel.org,
	tbird20d@gmail.com, frowand.list@gmail.com, Yamamoto, Masayuki,
	Nozawa, Hideki (STWN), Yonezawa, Kota, Matsumoto, Toshihiko,
	Watanabe, Satoshi (SSS)
In-Reply-To: <ECADFF3FD767C149AD96A924E7EA6EAF40AE4BB5@USCULXMSG01.am.sony.com>

Em Thu, 14 Dec 2017 17:32:34 +0000
"Bird, Timothy" <Tim.Bird@sony.com> escreveu:

> > -----Original Message-----
> > From: Philippe on Thursday, December 14, 2017 6:25 AM
> > Dear Mauro,
> > 
> > On Thu, Dec 14, 2017 at 11:55 AM, Mauro Carvalho Chehab
> > <mchehab@s-opensource.com> wrote:
> >   
> > > SPDX is a new requirement that started late on Kernel 4.14 development
> > > cycle (and whose initial changes were merged directly at Linus tree).
> > > Not all existing files have it yet, as identifying the right license
> > > on existing files is a complex task, but if you do a:
> > >
> > >         $ git grep SPDX $(find . -name Makefile) $(find . -name Kconfig)
> > >
> > > You'll see that lot of such files have it already.  
> > 
> > FWIW, short of having SPDX tags, identifying the right license on
> > existing files is not a super complex task: if boils down to running
> > many diffs.
> > 
> > Take the ~60K files in kernel, and about 6K license and notices
> > reference texts. Then compute a pairwise diff of each of the 60K file
> > against the 6K reference texts. Repeat the pairwise diff a few more
> > times, say 10 times, as multiple licenses may appear in any given
> > kernel file. And keep the diffs that have the fewest
> > difference/highest similarity with the reference texts as the detected
> > license. Done!  
> 
> You can't do license detection and assignment in this automated fashion - 
> at least not generally.
> 
> Even a single word of difference between the notice in the source
> code and the reference license notice or text may have legal implications
> that are not conveyed by the simplified SPDX tag.  When differences are
> found, we're going to have to kick the discrepancies to a human for review.
> This is especially true for files with multiple licenses.
> 
> For a work of original authorship, or a single copyright holder, the author
> or copyright holder may be able to change the notice or text, or gloss
> over any difference from the reference text, and make the SPDX  assignment
> (or even change the license, if they want).  This would apply to something
> new like this Sony driver.  However, for code that is already in the kernel
> tree, with likely multiple contributors, the legal situation gets a little
> more murky.

Precisely. This is easily fixable when the code author changes the
license text, or when someone from the Company that holds copyrights sends
the SPDX markups (as emails @company can be seen as official e-mails, except
if explicitly noticed otherwise). So, from my side, I'm now requiring
SPDX for new drivers.

However, if someone else is doing the changes, it can be tricky and risky
to pick up the patch, adding my SOB, if not endorsed by the copyright
owners, or by LF legal counseling. So, I prefer to not pick those myself,
except from people I trust.

> I suspect the vast majority of the ~60k files will probably fall neatly into an
> SPDX category, but I'm guessing a fair number (maybe hundreds) will require
> some review and discussion.

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH] arm64: dts: Remove leading 0x and 0s from bindings notation
From: Matthias Brugger @ 2017-12-14 18:03 UTC (permalink / raw)
  To: Mathieu Malaterre, Rob Herring
  Cc: Wei Xu, Mark Rutland, Catalin Marinas, Will Deacon, Andy Gross,
	David Brown, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	linux-arm-kernel, devicetree, linux-kernel, linux-mediatek,
	linux-arm-msm, linux-soc
In-Reply-To: <20171214165352.27902-1-malat@debian.org>



On 12/14/2017 05:53 PM, Mathieu Malaterre wrote:
[...]
> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 26396ef53bde..0446b122a6e2 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -249,7 +249,7 @@
>  			reg = <0 0x10005000 0 0x1000>;
>  		};
>  
> -		pio: pinctrl@0x10005000 {
> +		pio: pinctrl@10005000 {
>  			compatible = "mediatek,mt8173-pinctrl";
>  			reg = <0 0x1000b000 0 0x1000>;
>  			mediatek,pctl-regmap = <&syscfg_pctl_a>;

Acked-by: Matthias Brugger <matthias.bgg@gmail.com>

^ permalink raw reply

* Re: [RFC PATCH 2/2] leds: as3645a: Update LED label generation
From: Sakari Ailus @ 2017-12-14 18:03 UTC (permalink / raw)
  To: Dan Murphy
  Cc: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel,
	laurent.pinchart, devicetree, linux-kernel, linux-leds
In-Reply-To: <20171212215024.30116-2-dmurphy@ti.com>

Hi Dan,

Thanks for the patchset.

On Tue, Dec 12, 2017 at 03:50:24PM -0600, Dan Murphy wrote:
> Generate the LED label based off either the
> DT label node or off the I2C ID in the
> i2c device id struct.
> 
> If the label is used then this should denote
> the LED function.  As an example it would
> be as3645a:<function>
> 
>  Otherwise if the label is not
> used the LED label will be as3645a:flash and
> as3645a:indicator.

Which tree the patch is for? I see "id" is a u32 in what I have, and
also not defined in this scope.

> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
>  drivers/leds/leds-as3645a.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
> index f883616d9e60..197acd69ddcc 100644
> --- a/drivers/leds/leds-as3645a.c
> +++ b/drivers/leds/leds-as3645a.c
> @@ -526,10 +526,11 @@ static int as3645a_parse_node(struct as3645a *flash,
>  
>  	rval = of_property_read_string(flash->flash_node, "label", &name);
>  	if (!rval)
> -		strlcpy(names->flash, name, sizeof(names->flash));
> +		snprintf(names->flash, sizeof(names->flash), "%s:%s",
> +			 id->name, name);
>  	else
>  		snprintf(names->flash, sizeof(names->flash),
> -			 "%s:flash", node->name);
> +			 "%s:flash", id->name);
>  
>  	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
>  				    &cfg->flash_timeout_us);
> @@ -570,10 +571,11 @@ static int as3645a_parse_node(struct as3645a *flash,
>  
>  	rval = of_property_read_string(flash->indicator_node, "label", &name);
>  	if (!rval)
> -		strlcpy(names->indicator, name, sizeof(names->indicator));
> +		snprintf(names->indicator, sizeof(names->indicator), "%s:%s",
> +			 id->name, name);
>  	else
>  		snprintf(names->indicator, sizeof(names->indicator),
> -			 "%s:indicator", node->name);
> +			 "%s:indicator", id->name);
>  
>  	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
>  				    &cfg->indicator_max_ua);



-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi

^ permalink raw reply

* [PATCH v2 2/2] leds: as3645a: Fix line over 80 characters
From: Dan Murphy @ 2017-12-14 17:37 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	rpurdie-Fm38FmjxZ/leoWH0uzbU5w,
	jacek.anaszewski-Re5JQEeQqe8AvxtiuMwx3w, pavel-+ZI9xUNit7I,
	sakari.ailus-X3B1VOXEql0, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-leds-u79uwXL29TY76Z2rM5mHXA, Dan Murphy
In-Reply-To: <20171214173727.10070-1-dmurphy-l0cyMroinI0@public.gmane.org>

Fix the warning for line over 80 characters.

WARNING: line over 80 characters
363: FILE: drivers/leds/leds-as3645a.c:363:
	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);

Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---

v2 - Split the warning fixes into 2 patches and fixed commit message - 
https://patchwork.kernel.org/patch/10108325/

 drivers/leds/leds-as3645a.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index 49fdd7c0a6f6..f883616d9e60 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -360,7 +360,8 @@ static int as3645a_set_flash_brightness(struct led_classdev_flash *fled,
 {
 	struct as3645a *flash = fled_to_as3645a(fled);
 
-	flash->flash_current = as3645a_current_to_reg(flash, true, brightness_ua);
+	flash->flash_current = as3645a_current_to_reg(flash, true,
+						      brightness_ua);
 
 	return as3645a_set_current(flash);
 }
-- 
2.15.0.124.g7668cbc60

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH v2 1/2] leds: as3645a: Fix quoted string split warning
From: Dan Murphy @ 2017-12-14 17:37 UTC (permalink / raw)
  To: robh+dt, mark.rutland, rpurdie, jacek.anaszewski, pavel,
	sakari.ailus, laurent.pinchart
  Cc: devicetree, linux-kernel, linux-leds, Dan Murphy

Fix a warning for a split quoted string across
lines.

WARNING: quoted string split across lines
459: FILE: drivers/leds/leds-as3645a.c:459:
		dev_err(dev, "AS3645A not detected "
			"(model %d rfu %d)\n", model, rfu);

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Dan Murphy <dmurphy@ti.com>
---

v2 - Split the warning fixes into 2 patches and fixed commit message -
https://patchwork.kernel.org/patch/10108325/

 drivers/leds/leds-as3645a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index 9a257f969300..49fdd7c0a6f6 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -455,8 +455,8 @@ static int as3645a_detect(struct as3645a *flash)
 
 	/* Verify the chip model and version. */
 	if (model != 0x01 || rfu != 0x00) {
-		dev_err(dev, "AS3645A not detected "
-			"(model %d rfu %d)\n", model, rfu);
+		dev_err(dev, "AS3645A not detected (model %d rfu %d)\n",
+			model, rfu);
 		return -ENODEV;
 	}
 
-- 
2.15.0.124.g7668cbc60

^ permalink raw reply related

* [RESEND PATCH v2 15/15] arm64: dts: msm8996: db820c: Add sound card support
From: srinivas.kandagatla @ 2017-12-14 17:34 UTC (permalink / raw)
  To: Andy Gross, Mark Brown, linux-arm-msm, alsa-devel
  Cc: David Brown, Rob Herring, Mark Rutland, Liam Girdwood,
	Patrick Lai, Banajit Goswami, Jaroslav Kysela, Takashi Iwai,
	linux-soc, devicetree, linux-kernel, linux-arm-kernel, sboyd,
	Srinivas Kandagatla
In-Reply-To: <20171214173402.19074-1-srinivas.kandagatla@linaro.org>

From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

This patch adds hdmi sound card support to db820c via qdsp.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi |  5 +++++
 arch/arm64/boot/dts/qcom/msm8996.dtsi        | 33 ++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
index 9769053957af..b955769b100d 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
@@ -190,6 +190,11 @@
 		};
 	};
 
+	snd {
+		compatible = "qcom,apq8096-sndcard";
+		qcom,model = "DB820c";
+		iommus = <&lpass_q6_smmu 1>;
+	};
 
 	gpio_keys {
 		compatible = "gpio-keys";
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index a144cec7bb71..25c43fb8ab49 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -1262,6 +1262,7 @@
 
 				phys = <&hdmi_phy>;
 				phy-names = "hdmi_phy";
+				#sound-dai-cells = <0>;
 
 				ports {
 					#address-cells = <1>;
@@ -1297,6 +1298,33 @@
 					      "ref_clk";
 			};
 		};
+
+	        lpass_q6_smmu: arm,smmu-lpass_q6@1600000 {
+			compatible = "qcom,msm8996-smmu-v2";
+	                reg = <0x1600000 0x20000>;
+	                #iommu-cells = <1>;
+                        power-domains = <&gcc HLOS1_VOTE_LPASS_CORE_GDSC>;
+
+			#global-interrupts = <1>;
+		        interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 393 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 394 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 396 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 397 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 399 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 402 IRQ_TYPE_LEVEL_HIGH>,
+		                <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&gcc GCC_HLOS1_VOTE_LPASS_CORE_SMMU_CLK>,
+				 <&gcc GCC_HLOS1_VOTE_LPASS_ADSP_SMMU_CLK>;
+			clock-names = "iface", "bus";
+                        status = "okay";
+		};
 	};
 
 	adsp-pil {
@@ -1325,6 +1353,11 @@
 			qcom,ipc = <&apcs 16 8>;
 			qcom,smd-edge = <1>;
 			qcom,remote-pid = <2>;
+
+			apr {
+				compatible = "qcom,apr-msm8996";
+				qcom,smd-channels = "apr_audio_svc";
+			};
 		};
 	};
 
-- 
2.15.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox