* Re: [PATCH 1/4] input: Add support for the tm2 touchkey device driver
From: Rob Herring @ 2017-01-04 14:15 UTC (permalink / raw)
To: Jaechul Lee
Cc: Dmitry Torokhov, Mark Rutland, Catalin Marinas, Will Deacon,
Kukjin Kim, Krzysztof Kozlowski, Javier Martinez Canillas,
Andi Shyti, Chanwoo Choi, beomho.seo, galaxyra, linux-arm-kernel,
linux-input, devicetree, linux-kernel, linux-samsung-soc
In-Reply-To: <1483430237-26823-2-git-send-email-jcsing.lee@samsung.com>
On Tue, Jan 03, 2017 at 04:57:14PM +0900, Jaechul Lee wrote:
> This patch adds the binding description of the tm2 touchkey
> device driver.
>
> Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
> ---
> .../bindings/input/samsung,tm2-touchkey.txt | 27 ++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/samsung,tm2-touchkey.txt
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 1/2] Doc: devicetree: bindings: Add vendor prefix entry - lwn
From: Rob Herring @ 2017-01-04 14:16 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Mark Rutland, Russell King, Shawn Guo, Fabio Estevam,
linux-kernel, devicetree, linux-arm-kernel, Vladimir Zapolskiy,
Sascha Hauer, Zerlauth Karl (LWN), Lukasz Majewski
In-Reply-To: <1483440381-24268-1-git-send-email-lukma@denx.de>
On Tue, Jan 03, 2017 at 11:46:20AM +0100, Lukasz Majewski wrote:
> This patch adds entry for LWN - the Liebherr-Werk Nenzing GmbH company to
> vendor-prefixes.txt file.
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
> Changes for v3:
> - Update to v4.10-rc2
>
> Changes for v2:
> - New patch
> ---
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
I guess LWN.net will never make any devices...
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH V5 4/3] brcmfmac: use wiphy_read_of_freq_limits to respect extra limits
From: Rob Herring @ 2017-01-04 14:20 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Johannes Berg, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
Martin Blumenstingl, Felix Fietkau, Arend van Spriel,
Arnd Bergmann, devicetree-u79uwXL29TY76Z2rM5mHXA,
Rafał Miłecki
In-Reply-To: <20170103225715.14072-4-zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Tue, Jan 03, 2017 at 11:57:15PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>
> There are some devices (e.g. Netgear R8000 home router) with one chipset
> model used for different radios, some of them limited to subbands. NVRAM
> entries don't contain any extra info on such limitations and firmware
> reports full list of channels to us. We need to store extra limitation
> info on DT to support such devices properly.
This is the type of explanation that I'm looking for in the DT patch as
well as the examples of where this data comes from now. Certainly, I
think having per platform settings in userspace is not what you want.
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 5/5] i2c: mux: pca954x: Add irq_mask_en to delay enabling irqs
From: Peter Rosin @ 2017-01-04 14:21 UTC (permalink / raw)
To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483522197-38819-6-git-send-email-preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
On 2017-01-04 10:29, Phil Reid wrote:
> Unfortunately some hardware device will assert their irq line immediately
> on power on and provide no mechanism to mask the irq. As the i2c muxes
> provide no method to mask irq line this provides a work around by keeping
> the parent irq masked until enough device drivers have loaded to service
> all pending interrupts.
>
> For example the the ltc1760 assert its SMBALERT irq immediately on power
> on. With two ltc1760 attached to bus 0 & 1 on a pca954x mux when the first
> device is registered irq are enabled and fire continuously as the second
> device driver has not yet loaded. Setting this parameter to 0x3 while
> delay the irq being enabled until both devices are ready.
I think this also needs a comment in the code, including a description
of the limitations. If the interrupt is shared between two devices on
the same bus, you would have the exact same problem and this workaround
would be no good...
Overall, this series fixes the issues I had with the patch from half
a year ago or so. Thanks!
With nitpicks fixed,
Acked-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Cheers,
peda
> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index f2dba7c..16269e7 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -83,6 +83,7 @@ struct pca954x {
>
> struct irq_domain *irq;
> unsigned int irq_mask;
> + unsigned int irq_mask_en;
Spell it out: irq_mask_enable
> };
>
> /* Provide specs for the PCA954x types we know about */
> @@ -251,9 +252,12 @@ static void pca954x_irq_unmask(struct irq_data *idata)
> struct pca954x *data = irq_data_get_irq_chip_data(idata);
> unsigned int pos = idata->hwirq;
>
> - if (!data->irq_mask)
> + if (!data->irq_mask_en && !data->irq_mask)
> enable_irq(data->client->irq);
> data->irq_mask |= BIT(pos);
> + if (data->irq_mask_en &&
> + (data->irq_mask & data->irq_mask) == data->irq_mask_en)
> + enable_irq(data->client->irq);
> }
>
> static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
> @@ -369,6 +373,9 @@ static int pca954x_probe(struct i2c_client *client,
> idle_disconnect_dt = of_node &&
> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>
> + of_property_read_u32(of_node, "i2c-mux-irq-mask-en",
"nxp,irq-mask-enable"
> + &data->irq_mask_en);
> +
> ret = pca953x_irq_setup(muxc);
> if (ret)
> goto irq_setup_failed;
>
--
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 v2 12/19] media: imx: Add SMFC subdev driver
From: Vladimir Zapolskiy @ 2017-01-04 14:23 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-13-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a media entity subdevice driver for the i.MX Sensor Multi-FIFO
> Controller module. Video frames are received from the CSI and can
> be routed to various sinks including the i.MX Image Converter for
> scaling, color-space conversion, motion compensated deinterlacing,
> and image rotation.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/staging/media/imx/Makefile | 1 +
> drivers/staging/media/imx/imx-smfc.c | 739 +++++++++++++++++++++++++++++++++++
> 2 files changed, 740 insertions(+)
> create mode 100644 drivers/staging/media/imx/imx-smfc.c
>
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index 133672a..3559d7b 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -5,4 +5,5 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media.o
> obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-media-common.o
>
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
May be
obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o imx-smfc.o
>
> diff --git a/drivers/staging/media/imx/imx-smfc.c b/drivers/staging/media/imx/imx-smfc.c
> new file mode 100644
> index 0000000..565048c
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-smfc.c
> @@ -0,0 +1,739 @@
> +/*
> + * V4L2 Capture SMFC Subdev for Freescale i.MX5/6 SOC
> + *
> + * This subdevice handles capture of raw/unconverted video frames
> + * from the CSI, directly to memory via the Sensor Multi-FIFO Controller.
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
Please sort the list of headers alphabetically.
> +#include <media/imx.h>
> +#include "imx-media.h"
> +
[snip]
> +static irqreturn_t imx_smfc_eof_interrupt(int irq, void *dev_id)
> +{
> + struct imx_smfc_priv *priv = dev_id;
> + struct imx_media_dma_buf *done, *next;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->irqlock, flags);
spin_lock(&priv->irqlock) should be sufficient.
> +
> + if (priv->last_eof) {
> + complete(&priv->last_eof_comp);
> + priv->last_eof = false;
> + goto unlock;
> + }
> +
> + /* inform CSI of this EOF so it can monitor frame intervals */
> + v4l2_subdev_call(priv->src_sd, core, interrupt_service_routine,
> + 0, NULL);
> +
> + done = imx_media_dma_buf_get_active(priv->out_ring);
> + /* give the completed buffer to the sink */
> + if (!WARN_ON(!done))
> + imx_media_dma_buf_done(done, IMX_MEDIA_BUF_STATUS_DONE);
> +
> + /* priv->next buffer is now the active one */
> + imx_media_dma_buf_set_active(priv->next);
> +
> + /* bump the EOF timeout timer */
> + mod_timer(&priv->eof_timeout_timer,
> + jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
> +
> + if (ipu_idmac_buffer_is_ready(priv->smfc_ch, priv->ipu_buf_num))
> + ipu_idmac_clear_buffer(priv->smfc_ch, priv->ipu_buf_num);
> +
> + /* get next queued buffer */
> + next = imx_media_dma_buf_get_next_queued(priv->out_ring);
> +
> + ipu_cpmem_set_buffer(priv->smfc_ch, priv->ipu_buf_num, next->phys);
> + ipu_idmac_select_buffer(priv->smfc_ch, priv->ipu_buf_num);
> +
> + /* toggle IPU double-buffer index */
> + priv->ipu_buf_num ^= 1;
> + priv->next = next;
> +
> +unlock:
> + spin_unlock_irqrestore(&priv->irqlock, flags);
> + return IRQ_HANDLED;
> +}
> +
[snip]
> +
> +static const struct platform_device_id imx_smfc_ids[] = {
> + { .name = "imx-ipuv3-smfc" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, imx_smfc_ids);
> +
> +static struct platform_driver imx_smfc_driver = {
> + .probe = imx_smfc_probe,
> + .remove = imx_smfc_remove,
> + .id_table = imx_smfc_ids,
> + .driver = {
> + .name = "imx-ipuv3-smfc",
> + .owner = THIS_MODULE,
You can drop owner assignment.
> + },
> +};
> +module_platform_driver(imx_smfc_driver);
> +
> +MODULE_DESCRIPTION("i.MX SMFC subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx-ipuv3-smfc");
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 3/5] i2c: mux: pca954x: Add interrupt controller support
From: Peter Rosin @ 2017-01-04 14:26 UTC (permalink / raw)
To: Phil Reid, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <89efbe82-4530-043e-3469-ece0748d0150-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 2017-01-04 15:13, Peter Rosin wrote:
> On 2017-01-04 10:29, Phil Reid wrote:
>> Various muxes can aggregate multiple interrupts from each i2c bus.
>> All of the muxes with interrupt support combine the active low irq lines
>> using an internal 'and' function and generate a combined active low
>> output. The muxes do provide the ability to read a control register to
>> determine which irq is active. By making the mux an irq controller isr
>> can potentially be reduced by reading the status register and then only
>> calling the registered isr on that bus segment.
>>
>> As there is no irq masking on the mux irq are disabled until irq_unmask is
>> called at least once.
>
> Irqs are not my strong point, I would prefer if someone else also had
> a look. And there are some comments below...
>
> Cheers,
> peda
>
>> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> ---
>> drivers/i2c/muxes/i2c-mux-pca954x.c | 126 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 126 insertions(+)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index 981d145..f2dba7c 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -40,14 +40,19 @@
>> #include <linux/i2c.h>
>> #include <linux/i2c-mux.h>
>> #include <linux/i2c/pca954x.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>> #include <linux/pm.h>
>> #include <linux/slab.h>
>>
>> #define PCA954X_MAX_NCHANS 8
>>
>> +#define PCA954X_IRQ_OFFSET 4
>> +
>> enum pca_type {
>> pca_9540,
>> pca_9542,
>> @@ -62,6 +67,7 @@ enum pca_type {
>> struct chip_desc {
>> u8 nchans;
>> u8 enable; /* used for muxes only */
>> + u8 has_irq;
>> enum muxtype {
>> pca954x_ismux = 0,
>> pca954x_isswi
>> @@ -74,6 +80,9 @@ struct pca954x {
>> u8 last_chan; /* last register value */
>> u8 deselect;
>> struct i2c_client *client;
>> +
>> + struct irq_domain *irq;
>> + unsigned int irq_mask;
>> };
>>
>> /* Provide specs for the PCA954x types we know about */
>> @@ -86,19 +95,23 @@ struct pca954x {
>> [pca_9542] = {
>> .nchans = 2,
>> .enable = 0x4,
>> + .has_irq = 1,
>> .muxtype = pca954x_ismux,
>> },
>> [pca_9543] = {
>> .nchans = 2,
>> + .has_irq = 1,
>> .muxtype = pca954x_isswi,
>> },
>> [pca_9544] = {
>> .nchans = 4,
>> .enable = 0x4,
>> + .has_irq = 1,
>> .muxtype = pca954x_ismux,
>> },
>> [pca_9545] = {
>> .nchans = 4,
>> + .has_irq = 1,
>> .muxtype = pca954x_isswi,
>> },
>> [pca_9547] = {
>> @@ -203,6 +216,104 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>> return pca954x_reg_write(muxc->parent, client, data->last_chan);
>> }
>>
>> +static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>> +{
>> + struct pca954x *data = dev_id;
>> + unsigned int child_irq;
>> + int ret, i, handled;
>> +
>> + ret = i2c_smbus_read_byte(data->client);
>> + if (ret < 0)
>> + return IRQ_NONE;
>> +
>> + for (i = 0; i < data->chip->nchans; i++) {
>> + if (ret & BIT(PCA954X_IRQ_OFFSET+i)) {
>
> Spaces around the +
>
>> + child_irq = irq_linear_revmap(data->irq, i);
>> + handle_nested_irq(child_irq);
>> + handled++;
>> + }
>> + }
>> + return handled ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static void pca954x_irq_mask(struct irq_data *idata)
>> +{
>> + struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> + unsigned int pos = idata->hwirq;
>> +
>> + data->irq_mask &= ~BIT(pos);
>> + if (!data->irq_mask)
>> + disable_irq(data->client->irq);
>> +}
>> +
>> +static void pca954x_irq_unmask(struct irq_data *idata)
>> +{
>> + struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> + unsigned int pos = idata->hwirq;
>> +
>> + if (!data->irq_mask)
>> + enable_irq(data->client->irq);
>> + data->irq_mask |= BIT(pos);
>> +}
>> +
>> +static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
>> +{
>> + if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
>> + return -EINVAL;
>> + return 0;
>> +}
>> +
>> +static struct irq_chip pca954x_irq_chip = {
>> + .name = "i2c-mux-pca954x",
>> + .irq_mask = pca954x_irq_mask,
>> + .irq_unmask = pca954x_irq_unmask,
>> + .irq_set_type = pca954x_irq_set_type,
>> +};
>> +
>> +static int pca953x_irq_setup(struct i2c_mux_core *muxc)
Ooop,
pca954x_irq_setup
>> +{
>> + struct pca954x *data = i2c_mux_priv(muxc);
>> + struct i2c_client *client = data->client;
>> + int c, err, irq;
>> +
>> + if (!data->chip->has_irq || client->irq <= 0)
>> + return 0;
>> +
>> + data->irq = irq_domain_add_linear(client->dev.of_node,
>> + data->chip->nchans,
>> + &irq_domain_simple_ops, data);
>> + if (!data->irq) {
>> + err = -ENODEV;
>> + goto err_irq_domain;
>
> This label is not needed, just return -ENODEV
>
>> + }
>> +
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_create_mapping(data->irq, c);
>> + irq_set_chip_data(irq, data);
>> + irq_set_chip_and_handler(irq, &pca954x_irq_chip,
>> + handle_simple_irq);
>> + }
>> +
>> + err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
>> + pca954x_irq_handler,
>> + IRQF_ONESHOT | IRQF_SHARED,
>> + "pca953x", data);
>
> "pca954x"
>
>> + if (err)
>> + goto err_req_irq;
>> +
>> + disable_irq(data->client->irq);
>> +
>> + return 0;
>> +err_req_irq:
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_find_mapping(data->irq, c);
>> + irq_dispose_mapping(irq);
>> + }
>> + irq_domain_remove(data->irq);
>> +err_irq_domain:
>> + return err;
>> +}
>> +
>> /*
>> * I2C init/probing/exit functions
>> */
>> @@ -258,6 +369,10 @@ static int pca954x_probe(struct i2c_client *client,
>> idle_disconnect_dt = of_node &&
>> of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>>
>> + ret = pca953x_irq_setup(muxc);
>> + if (ret)
>> + goto irq_setup_failed;
>> +
>> /* Now create an adapter for each channel */
>> for (num = 0; num < data->chip->nchans; num++) {
>> bool idle_disconnect_pd = false;
>> @@ -294,6 +409,7 @@ static int pca954x_probe(struct i2c_client *client,
>>
>> return 0;
>>
>> +irq_setup_failed:
>> virt_reg_failed:
>
> Rename the virt_reg_failed label to name what is reversed instead of what
> happened to fail. E.g. fail_del_adapters, or something.
>
>> i2c_mux_del_adapters(muxc);
>> return ret;
>> @@ -302,6 +418,16 @@ static int pca954x_probe(struct i2c_client *client,
>> static int pca954x_remove(struct i2c_client *client)
>> {
>> struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> + struct pca954x *data = i2c_mux_priv(muxc);
>> + int c, irq;
>> +
>> + if (data->irq) {
>> + for (c = 0; c < data->chip->nchans; c++) {
>> + irq = irq_find_mapping(data->irq, c);
>> + irq_dispose_mapping(irq);
>> + }
>> + irq_domain_remove(data->irq);
>> + }
>>
>> i2c_mux_del_adapters(muxc);
>> return 0;
>>
>
--
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] iio: adc: Add Renesas GyroADC driver
From: Marek Vasut @ 2017-01-04 14:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Geert Uytterhoeven, Simon Horman
In-Reply-To: <CAMuHMdX20kc8h4DiNLu0KwaA0FL6F-WXLnA3YXvu02VJTh_7fQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 01/02/2017 11:01 AM, Geert Uytterhoeven wrote:
> Hi Marek,
>
> On Fri, Dec 30, 2016 at 8:18 PM, Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Add IIO driver for the Renesas RCar GyroADC block. This block is a
>> simple 4/8-channel ADC which samples 12/15/24 bits of data every
>> cycle from all channels.
>
> Thanks for your patch!
>
>> Signed-off-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>> Cc: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
>> ---
>> .../bindings/iio/adc/renesas,gyroadc.txt | 38 +++
>> MAINTAINERS | 6 +
>> drivers/iio/adc/Kconfig | 10 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/rcar_gyro_adc.c | 379 +++++++++++++++++++++
>> 5 files changed, 434 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>> create mode 100644 drivers/iio/adc/rcar_gyro_adc.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>> new file mode 100644
>> index 0000000..3fd5f57
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>> @@ -0,0 +1,38 @@
>> +* Renesas RCar GyroADC device driver
>> +
>> +Required properties:
>> +- compatible: Should be "renesas,rcar-gyroadc" for regular GyroADC or
>> + "renesas,rcar-gyroadc-r8a7792" for GyroADC without interrupt
>
> Please use "renesas,r8a7792-gyroadc" to match existing practices.
Actually, that should probably be gyroadc-r8a7791 if we want to match
the existing practice. Fixed.
> Unfortunately we cannot just look at the presence of an "interrupts" property
> to distinguish between the two variants, as the interrupt is handled
> by a different
> block (Speed Pulse IF).
>
> Do you have a (future) use for the interrupt? If yes, we will need a phandle
> link to the Speed Pulse IF device node later...
Nope, I don't have any use for it yet and I doubt it'll come useful later.
> BTW, it's a good idea to always cater for SoC-specific differences that may
> be discovered later by adding SoC-specific compatible values.
Yep
> BTW, the same hardware block seems to be present in R-Car Gen1 and
> R-Car Gen3 SoCs.
OK
>> + block found in R8A7792.
>
>> +- renesas,gyroadc-vref: Array of reference voltage values for each input to
>> + the GyroADC, in uV. Array must have 4 elemenets for
>> + mode 1 and 8 elements for mode 2 and 3.
>
> Should this be an array of phandles to regulators instead?
That's a good idea, yeah.
>> --- /dev/null
>> +++ b/drivers/iio/adc/rcar_gyro_adc.c
>> @@ -0,0 +1,379 @@
>> +/*
>> + * Renesas RCar GyroADC driver
>
> R-Car
>
>> + *
>> + * Copyright 2016 Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>
> MODULE_LICENSE() says GPL V2 only?
Fixed to GPL.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/interrupt.h>
>
> Do you need this include? There's no interrupt support.
Nope
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/slab.h>
>> +#include <linux/io.h>
>> +#include <linux/clk.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/regulator/consumer.h>
>
> Do you need this include?
> Ah, you were already anticipating the array of phandles to regulators ;-)
Of course ... :)
>> +struct rcar_gyroadc {
>> + struct device *dev;
>> + void __iomem *regs;
>> + struct clk *fclk;
>> + struct clk *clk;
>
> iclk?
OK
>> +static void rcar_gyroadc_hw_init(struct rcar_gyroadc *priv)
>> +{
>> + unsigned long clk_mhz = clk_get_rate(priv->clk) / 1000000;
>> +
>> + /* Stop the GyroADC. */
>> + writel(0, priv->regs + RCAR_GYROADC_START_STOP);
>> +
>> + /* Disable IRQ, except on V2H. */
>> + if (priv->model != RCAR_GYROADC_MODEL_R8A7792)
>> + writel(0, priv->regs + RCAR_GYROADC_INTENR);
>> +
>> + /* Set mode and timing. */
>> + writel(priv->mode, priv->regs + RCAR_GYROADC_MODE_SELECT);
>> +
>> + if (priv->mode == RCAR_GYROADC_MODE_SELECT_1_MB88101A)
>> + writel(clk_mhz * 10, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
>> + else if (priv->mode == RCAR_GYROADC_MODE_SELECT_2_ADCS7476)
>> + writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
>> + else if (priv->mode == RCAR_GYROADC_MODE_SELECT_3_MAX1162)
>> + writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
>> + writel(clk_mhz * 1250, priv->regs + RCAR_GYROADC_1_25MS_LENGTH);
>> +
>> + /*
>> + * We can possibly turn the sampling on/off on-demand to reduce power
>
> And the module clock, using runtime PM (see below).
>
>> + * consumption, but for the sake of quick availability of samples, we
>> + * don't do it now.
>> + */
>> + writel(RCAR_GYROADC_START_STOP_START,
>> + priv->regs + RCAR_GYROADC_START_STOP);
>> +
>> + /* Wait for the first conversion to complete. */
>> + udelay(1250);
>> +}
>
>> +static const struct of_device_id rcar_gyroadc_match[] = {
>> + {
>> + /* RCar Gen2 compatible GyroADC */
>
> R-Car
>
>> + .compatible = "renesas,rcar-gyroadc",
>> + .data = (void *)RCAR_GYROADC_MODEL_DEFAULT,
>> + }, {
>> + /* RCar V2H specialty without interrupt registers. */
>
> R-Car
>
>> + .compatible = "renesas,rcar-gyroadc-r8a7792",
>> + .data = (void *)RCAR_GYROADC_MODEL_R8A7792,
>> + }, {
>> + /* sentinel */
>> + }
>> +};
>
>> +static int rcar_gyroadc_probe(struct platform_device *pdev)
>> +{
>
>> + priv->fclk = devm_clk_get(dev, "fck");
>
> The module clock isn't used directly by this driver (you don't need
> e.g. its rate),
> so you can leave out all handling related to this clock iff you enable
> runtime PM:
>
> pm_runtime_enable(dev);
> pm_runtime_get_sync(dev);
>
> Then runtime PM will take care of enabling the module clock, as the
> GyroADC block is part of the CPG/MSSR clock domain.
> Doing that also means the driver keeps on working in future SoCs where
> the GyroADC block may be located in a power area.
So I won't even need the fclk phandle in DT, right ?
>> + if (IS_ERR(priv->fclk)) {
>> + ret = PTR_ERR(priv->fclk);
>> + dev_err(dev, "Failed to get FCK clock (ret=%i)\n", ret);
>
> Do you need this error message? It will add to the noise in case of
> deferred probing.
> Please either remove it, or make it depend on != -EPROBE_DEFER.
OK
>> + return ret;
>> + }
>> +
>> + priv->clk = devm_clk_get(dev, "if");
>> + if (IS_ERR(priv->clk)) {
>> + ret = PTR_ERR(priv->clk);
>> + dev_err(dev, "Failed to get IF clock (ret=%i)\n", ret);
>
> Likewise.
OK
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(pdev->dev.of_node, "renesas,gyroadc-mode",
>> + &mode);
>> + if (ret || (mode < 1) || (mode > 3)) {
>> + dev_err(dev, "Failed to get GyroADC mode (ret=%i)\n", ret);
>
> Note that this will print "ret=0" (no error?) if an invalid mode was specified.
Oh, nice.
>> + return ret;
>> + }
>> +
>> + if (mode == 1)
>> + priv->mode = RCAR_GYROADC_MODE_SELECT_1_MB88101A;
>> + else if (mode == 2)
>> + priv->mode = RCAR_GYROADC_MODE_SELECT_2_ADCS7476;
>> + else if (mode == 3)
>> + priv->mode = RCAR_GYROADC_MODE_SELECT_3_MAX1162;
>
> switch()? And print the invalid mode message here?
>
>> +
>> + of_property_read_u32_array(pdev->dev.of_node, "renesas,gyroadc-vref",
>> + priv->vref_uv, (mode == 1) ? 4 : 8);
>
> This call may fail.
Both reworked, thanks for the review.
--
Best regards,
Marek Vasut
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Kalle Valo @ 2017-01-04 14:32 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rob Herring, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CACna6rxmgN2zz-Fc1-Uu02oqMQreNkMJt0jGjLOMrXM=6iujFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>
>>> This new file should be used for properties handled at higher level and
>>> so usable with all drivers.
>>
>> Why is this needed? Where would this data normally come from?
>
> Vendors limit choice of channels at their web user interface level. I
> want to do better and report proper channels directly at kernel level
> instead of masking them in every possible configuration tool.
Why do vendors limit the channels? Is it because of a hardware
limitation (antenna does not support, or not calibrated, for a certain
band etc) or something else?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: gpio: Add binding documentation for gpio-thunderx
From: Rob Herring @ 2017-01-04 14:33 UTC (permalink / raw)
To: David Daney
Cc: Linus Walleij, Alexandre Courbot, Mark Rutland, linux-gpio,
devicetree, linux-kernel, David Daney
In-Reply-To: <1483491334-167095-2-git-send-email-ddaney.cavm@gmail.com>
On Tue, Jan 03, 2017 at 04:55:32PM -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
>
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
> .../devicetree/bindings/gpio/gpio-thunderx.txt | 33 ++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-thunderx.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-thunderx.txt b/Documentation/devicetree/bindings/gpio/gpio-thunderx.txt
> new file mode 100644
> index 0000000..ba3cdae
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-thunderx.txt
> @@ -0,0 +1,33 @@
> +Cavium ThunderX/OCTEON-TX GPIO controller bindings
> +
> +Required Properties:
> +- reg: The controller bus address.
> +- gpio-controller: Marks the device node as a GPIO controller.
> +- #gpio-cells: Must be 2.
> + - First cell is the GPIO pin number relative to the controller.
> + - Second cell is standard of_gpio_flags:
> + 1 - Active Low.
> + 2 - Single Ended.
Just reference where these are defined.
> +
> +Optional Properties:
> +- compatible: "cavium,thunder-8890-gpio", unused as PCI driver binding is used.
> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells: Must be present and have value of 2 if
> + "interrupt-controller" is present.
> + - First cell is the GPIO pin number relative to the controller.
> + - Second cell is triggering flags, one of:
> + 1 - Edge Rising
> + 2 - Edge Falling
> + 4 - Level High
> + 8 - Level Low
Just reference interrupt-controller/interrupts.txt or the header
defining these.
> +
> +Example:
> +
> +gpio_6_0: gpio0@6,0 {
gpio@6,0
> + compatible = "cavium,thunder-8890-gpio";
> + reg = <0x3000 0 0 0 0>; /* DEVFN = 0x30 (6:0) */
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> +};
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v3 2/2] dt-bindings: Add DT bindings info for FlexRM ring manager
From: Rob Herring @ 2017-01-04 14:37 UTC (permalink / raw)
To: Anup Patel
Cc: Jassi Brar, Mark Rutland, Ray Jui, Scott Branden, Pramod KUMAR,
Rob Rice, devicetree, linux-kernel, linux-arm-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1483508082-7008-3-git-send-email-anup.patel@broadcom.com>
On Wed, Jan 04, 2017 at 11:04:42AM +0530, Anup Patel wrote:
> This patch adds device tree bindings document for the FlexRM
> ring manager found on Broadcom iProc SoCs.
>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
> ---
> .../bindings/mailbox/brcm,iproc-flexrm-mbox.txt | 60 ++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
>
> diff --git a/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> new file mode 100644
> index 0000000..ca51a39
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/brcm,iproc-flexrm-mbox.txt
> @@ -0,0 +1,60 @@
> +Broadcom FlexRM Ring Manager
> +============================
> +The Broadcom FlexRM ring manager provides a set of rings which can be
> +used to submit work to offload engines. An SoC may have multiple FlexRM
> +hardware blocks. There is one device tree entry per FlexRM block. The
> +FlexRM driver will create a mailbox-controller instance for given FlexRM
> +hardware block where each mailbox channel is a separate FlexRM ring.
> +
> +Required properties:
> +--------------------
> +- compatible: Should be "brcm,iproc-flexrm-mbox"
> +- reg: Specifies base physical address and size of the FlexRM
> + ring registers
> +- msi-parent: Phandles (and potential Device IDs) to MSI controllers
> + The FlexRM engine will send MSIs (instead of wired
> + interrupts) to CPU. There is one MSI for each FlexRM ring.
> + Refer devicetree/bindings/interrupt-controller/msi.txt
> +- #mbox-cells: Specifies the number of cells needed to encode a mailbox
> + channel. This should be 3.
> +
> + The 1st cell is the mailbox channel number.
> +
> + The 2nd cell contains MSI completion threshold. This is the
> + number of completion messages for which FlexRM will inject
> + one MSI interrupt to CPU.
> +
> + The 3nd cell contains MSI timer value representing time for
> + which FlexRM will wait to accumulate N completion messages
> + where N is the value specified by 2nd cell above. If FlexRM
> + does not get required number of completion messages in time
> + specified by this cell then it will inject one MSI interrupt
> + to CPU provided atleast one completion message is available.
> +
> +Optional properties:
> +--------------------
> +- dma-coherent: Present if DMA operations made by the FlexRM engine (such
> + as DMA descriptor access, access to buffers pointed by DMA
> + descriptors and read/write pointer updates to DDR) are
> + cache coherent with the CPU.
> +
> +Example:
> +--------
> +crypto_mbox: mbox@67000000 {
> + compatible = "brcm,iproc-flexrm-mbox";
> + reg = <0x67000000 0x200000>;
> + msi-parent = <&gic_its 0x7f00>;
> + #mbox-cells = <3>;
> +};
> +
> +crypto_client {
crypto@<addr>
> + ...
> + mboxes = <&crypto_mbox 0 0x1 0xffff>,
> + <&crypto_mbox 1 0x1 0xffff>,
> + <&crypto_mbox 16 0x1 0xffff>,
> + <&crypto_mbox 17 0x1 0xffff>,
> + <&crypto_mbox 30 0x1 0xffff>,
> + <&crypto_mbox 31 0x1 0xffff>;
> + };
> + ...
Please somewhat fully list the contents for node.
Rob
^ permalink raw reply
* Re: [PATCH resend v4 2/3] ARM: dts: imx6: Support Savageboard dual
From: Rob Herring @ 2017-01-04 14:38 UTC (permalink / raw)
To: Milo Kim
Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <20170104070436.3425-1-woogyom.kim@gmail.com>
On Wed, Jan 04, 2017 at 04:04:36PM +0900, Milo Kim wrote:
> Common savageboard DT file is used for board support.
> Add the vendor name and specify the dtb file for i.MX6Q build.
>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/imx6dl-savageboard.dts | 51 ++++++++++++++++++++++
> 3 files changed, 53 insertions(+)
> create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v5 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board
From: Rob Herring @ 2017-01-04 14:44 UTC (permalink / raw)
To: Hoegeun Kwon
Cc: devicetree, krzk, cw00.choi, Donghwa Lee, jh80.chung,
linux-kernel, dri-devel, linux-samsung-soc, kgene, Hyungwon Hwang
In-Reply-To: <1483517711-23849-3-git-send-email-hoegeun.kwon@samsung.com>
On Wed, Jan 04, 2017 at 05:15:10PM +0900, Hoegeun Kwon wrote:
> This patch add support for MIPI-DSI based S6E3HA2 AMOLED panel
> driver. This panel has 1440x2560 resolution in 5.7-inch physical
> panel in the TM2 device.
>
> Signed-off-by: Donghwa Lee <dh09.lee@samsung.com>
> Signed-off-by: Hyungwon Hwang <human.hwang@samsung.com>
> Signed-off-by: Hoegeun Kwon <hoegeun.kwon@samsung.com>
> ---
> .../bindings/display/panel/samsung,s6e3ha2.txt | 40 ++
> drivers/gpu/drm/panel/Kconfig | 6 +
> drivers/gpu/drm/panel/Makefile | 1 +
> drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c | 741 +++++++++++++++++++++
> 4 files changed, 788 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> create mode 100644 drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c
>
> diff --git a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> new file mode 100644
> index 0000000..6879f51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha2.txt
> @@ -0,0 +1,40 @@
> +Samsung S6E3HA2 5.7" 1440x2560 AMOLED panel
> +
> +Required properties:
> + - compatible: "samsung,s6e3ha2"
> + - reg: the virtual channel number of a DSI peripheral
> + - vdd3-supply: I/O voltage supply
> + - vci-supply: voltage supply for analog circuits
> + - reset-gpios: a GPIO spec for the reset pin (active low)
> + - enable-gpios: a GPIO spec for the panel enable pin (active high)
> + - te-gpios: a GPIO spec for the tearing effect synchronization signal
> + gpio pin (active high)
> +
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in [1]. This
> +node should describe panel's video bus.
> +
> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Example:
> +
> +&dsi {
> + ...
> +
> + panel@0 {
> + compatible = "samsung,s6e3ha2";
> + reg = <0>;
> + vdd3-supply = <&ldo27_reg>;
> + vci-supply = <&ldo28_reg>;
> + reset-gpios = <&gpg0 0 GPIO_ACTIVE_LOW>;
> + enable-gpios = <&gpf1 5 GPIO_ACTIVE_HIGH>;
> + te-gpios = <&gpf1 3 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&dsi_out>;
As I said previously, it makes no sense to have a graph to dsi_out it is
simply the parent node.
> + };
> + };
> + };
> +};
> +
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 2/5] dt: bindings: i2c-mux-pca954x: Add documentation for interrupt controller
From: Rob Herring @ 2017-01-04 14:45 UTC (permalink / raw)
To: Phil Reid; +Cc: peda, wsa, mark.rutland, linux-i2c, devicetree
In-Reply-To: <1483522197-38819-3-git-send-email-preid@electromag.com.au>
On Wed, Jan 04, 2017 at 05:29:54PM +0800, Phil Reid wrote:
> Various muxes can aggregate multiple irq lines and provide a control
> register to determine the active line. Add bindings for interrupt
> controller support.
>
> Signed-off-by: Phil Reid <preid@electromag.com.au>
> ---
> Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> index cf53d5f..9f7c275 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-pca954x.txt
> @@ -19,7 +19,14 @@ Optional Properties:
> - i2c-mux-idle-disconnect: Boolean; if defined, forces mux to disconnect all
> children in idle state. This is necessary for example, if there are several
> multiplexers on the bus and the devices behind them use same I2C addresses.
> -
> + - interrupt-parent: Phandle for the interrupt controller that services
> + interrupts for this device.
> + - interrupts: Interrupt mapping for IRQ.
> + - interrupt-controller: Marks the device node as a interrupt controller.
> + - #interrupt-cells : Should be two.
> + - first cell is the pin number
> + - second cell is used to specify flags.
> + See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>
> Example:
>
> @@ -29,6 +36,11 @@ Example:
> #size-cells = <0>;
> reg = <0x74>;
>
> + interrupt-parent = <&ipic>;
> + interrupts = <17 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-controller;
> + #interrupt-cells=<2>;
Needs spaces around the '='. With that,
Acked-by: Rob Herring <robh@kernel.org>
> +
> i2c@2 {
> #address-cells = <1>;
> #size-cells = <0>;
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [PATCH v2 13/19] media: imx: Add IC subdev drivers
From: Vladimir Zapolskiy @ 2017-01-04 14:48 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-14-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is a set of three media entity subdevice drivers for the i.MX
> Image Converter. The i.MX IC module contains three independent
> "tasks":
>
> - Pre-processing Encode task: video frames are routed directly from
> the CSI and can be scaled, color-space converted, and rotated.
> Scaled output is limited to 1024x1024 resolution. Output frames
> are routed to the camera interface entities (camif).
>
> - Pre-processing Viewfinder task: this task can perform the same
> conversions as the pre-process encode task, but in addition can
> be used for hardware motion compensated deinterlacing. Frames can
> come either directly from the CSI or from the SMFC entities (memory
> buffers via IDMAC channels). Scaled output is limited to 1024x1024
> resolution. Output frames can be routed to various sinks including
> the post-processing task entities.
>
> - Post-processing task: same conversions as pre-process encode. However
> this entity sends frames to the i.MX IPU image converter which supports
> image tiling, which allows scaled output up to 4096x4096 resolution.
> Output frames can be routed to the camera interfaces.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
[snip]
> +static int imx_ic_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct imx_ic_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, &priv->sd);
> + priv->dev = &pdev->dev;
> +
> + /* get our ipu_id, grp_id and IC task id */
> + pdata = priv->dev->platform_data;
> + priv->ipu_id = pdata->ipu_id;
> + switch (pdata->grp_id) {
> + case IMX_MEDIA_GRP_ID_IC_PRPENC:
> + priv->task_id = IC_TASK_ENCODER;
> + break;
> + case IMX_MEDIA_GRP_ID_IC_PRPVF:
> + priv->task_id = IC_TASK_VIEWFINDER;
> + break;
> + case IMX_MEDIA_GRP_ID_IC_PP0...IMX_MEDIA_GRP_ID_IC_PP3:
> + priv->task_id = IC_TASK_POST_PROCESSOR;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + v4l2_subdev_init(&priv->sd, ic_ops[priv->task_id]->subdev_ops);
> + v4l2_set_subdevdata(&priv->sd, priv);
> + priv->sd.internal_ops = ic_ops[priv->task_id]->internal_ops;
> + priv->sd.entity.ops = ic_ops[priv->task_id]->entity_ops;
> + priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_SCALER;
> + priv->sd.dev = &pdev->dev;
> + priv->sd.owner = THIS_MODULE;
> + priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> + priv->sd.grp_id = pdata->grp_id;
> + strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
> +
> + ret = ic_ops[priv->task_id]->init(priv);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_async_register_subdev(&priv->sd);
> + if (ret)
> + goto remove;
> +
> + return 0;
> +remove:
> + ic_ops[priv->task_id]->remove(priv);
> + return ret;
if (ret)
ic_ops[priv->task_id]->remove(priv);
return ret;
as an alternative.
[snip]
> +static const struct platform_device_id imx_ic_ids[] = {
> + { .name = "imx-ipuv3-ic" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, imx_ic_ids);
> +
> +static struct platform_driver imx_ic_driver = {
> + .probe = imx_ic_probe,
> + .remove = imx_ic_remove,
> + .id_table = imx_ic_ids,
> + .driver = {
> + .name = "imx-ipuv3-ic",
> + .owner = THIS_MODULE,
Please drop .owner assignment.
> + },
> +};
> +module_platform_driver(imx_ic_driver);
> +
> +MODULE_DESCRIPTION("i.MX IC subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:imx-ipuv3-ic");
> diff --git a/drivers/staging/media/imx/imx-ic-pp.c b/drivers/staging/media/imx/imx-ic-pp.c
> new file mode 100644
> index 0000000..5ef0581
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-ic-pp.c
> @@ -0,0 +1,636 @@
> +/*
> + * V4L2 IC Post-Processor Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2014-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
Please sort the list of headers alphabetically.
> +#include <media/imx.h>
> +#include <video/imx-ipu-image-convert.h>
> +#include "imx-media.h"
> +#include "imx-ic.h"
> +
[snip]
> +
> +static int pp_start(struct pp_priv *priv)
> +{
> + struct imx_ic_priv *ic_priv = priv->ic_priv;
> + struct ipu_image image_in, image_out;
> + const struct imx_media_pixfmt *incc;
> + struct v4l2_mbus_framefmt *infmt;
> + int i, in_size, ret;
> +
> + /* ask the sink for the buffer ring */
> + ret = v4l2_subdev_call(priv->sink_sd, core, ioctl,
> + IMX_MEDIA_REQ_DMA_BUF_SINK_RING,
> + &priv->out_ring);
> + if (ret)
> + return ret;
> +
> + imx_media_mbus_fmt_to_ipu_image(&image_in,
> + &priv->format_mbus[priv->input_pad]);
> + imx_media_mbus_fmt_to_ipu_image(&image_out,
> + &priv->format_mbus[priv->output_pad]);
> +
> + priv->ipu = priv->md->ipu[ic_priv->ipu_id];
> + priv->ic_ctx = ipu_image_convert_prepare(priv->ipu,
> + IC_TASK_POST_PROCESSOR,
> + &image_in, &image_out,
> + priv->rot_mode,
> + pp_convert_complete, priv);
> + if (IS_ERR(priv->ic_ctx))
> + return PTR_ERR(priv->ic_ctx);
> +
> + infmt = &priv->format_mbus[priv->input_pad];
> + incc = priv->cc[priv->input_pad];
> + in_size = (infmt->width * incc->bpp * infmt->height) >> 3;
> +
> + if (priv->in_ring) {
> + v4l2_warn(&ic_priv->sd, "%s: dma-buf ring was not freed\n",
> + __func__);
> + imx_media_free_dma_buf_ring(priv->in_ring);
> + }
> +
> + priv->in_ring = imx_media_alloc_dma_buf_ring(priv->md,
> + &priv->src_sd->entity,
> + &ic_priv->sd.entity,
> + in_size,
> + IMX_MEDIA_MIN_RING_BUFS,
> + true);
> + if (IS_ERR(priv->in_ring)) {
> + v4l2_err(&ic_priv->sd,
> + "failed to alloc dma-buf ring\n");
> + ret = PTR_ERR(priv->in_ring);
> + priv->in_ring = NULL;
> + goto out_unprep;
> + }
> +
> + for (i = 0; i < IMX_MEDIA_MIN_RING_BUFS; i++)
> + imx_media_dma_buf_queue(priv->in_ring, i);
> +
> + priv->out_run = kzalloc(IMX_MEDIA_MAX_RING_BUFS *
> + sizeof(*priv->out_run), GFP_KERNEL);
> + if (!priv->out_run) {
> + v4l2_err(&ic_priv->sd, "failed to alloc src ring runs\n");
In OOM situation the core will report it, probably you can drop the message.
> + ret = -ENOMEM;
> + goto out_free_ring;
> + }
> +
> + priv->stop = false;
> +
> + return 0;
> +
> +out_free_ring:
> + imx_media_free_dma_buf_ring(priv->in_ring);
> + priv->in_ring = NULL;
> +out_unprep:
> + ipu_image_convert_unprepare(priv->ic_ctx);
> + return ret;
> +}
> +
[snip]
> diff --git a/drivers/staging/media/imx/imx-ic-prpenc.c b/drivers/staging/media/imx/imx-ic-prpenc.c
> new file mode 100644
> index 0000000..e17216b
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-ic-prpenc.c
> @@ -0,0 +1,1037 @@
> +/*
> + * V4L2 Capture IC Encoder Subdev for Freescale i.MX5/6 SOC
> + *
> + * This subdevice handles capture of video frames from the CSI, which
> + * are routed directly to the Image Converter preprocess encode task,
> + * for resizing, colorspace conversion, and rotation.
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
Please sort the list of headers alphabetically.
> +#include <media/imx.h>
> +#include "imx-media.h"
> +#include "imx-ic.h"
> +
[snip]
> +static irqreturn_t prpenc_eof_interrupt(int irq, void *dev_id)
> +{
> + struct prpenc_priv *priv = dev_id;
> + struct imx_media_dma_buf *done, *next;
> + struct ipuv3_channel *channel;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->irqlock, flags);
Here spin_lock(&priv->irqlock) should be sufficient.
> +
> + if (priv->last_eof) {
> + complete(&priv->last_eof_comp);
> + priv->last_eof = false;
> + goto unlock;
> + }
> +
> + /* inform CSI of this EOF so it can monitor frame intervals */
> + v4l2_subdev_call(priv->src_sd, core, interrupt_service_routine,
> + 0, NULL);
> +
> + channel = (ipu_rot_mode_is_irt(priv->rot_mode)) ?
> + priv->enc_rot_out_ch : priv->enc_ch;
> +
> + done = imx_media_dma_buf_get_active(priv->out_ring);
> + /* give the completed buffer to the sink */
> + if (!WARN_ON(!done))
> + imx_media_dma_buf_done(done, IMX_MEDIA_BUF_STATUS_DONE);
> +
> + /* priv->next buffer is now the active one */
> + imx_media_dma_buf_set_active(priv->next);
> +
> + /* bump the EOF timeout timer */
> + mod_timer(&priv->eof_timeout_timer,
> + jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
> +
> + if (ipu_idmac_buffer_is_ready(channel, priv->ipu_buf_num))
> + ipu_idmac_clear_buffer(channel, priv->ipu_buf_num);
> +
> + /* get next queued buffer */
> + next = imx_media_dma_buf_get_next_queued(priv->out_ring);
> +
> + ipu_cpmem_set_buffer(channel, priv->ipu_buf_num, next->phys);
> + ipu_idmac_select_buffer(channel, priv->ipu_buf_num);
> +
> + /* toggle IPU double-buffer index */
> + priv->ipu_buf_num ^= 1;
> + priv->next = next;
> +
> +unlock:
> + spin_unlock_irqrestore(&priv->irqlock, flags);
> + return IRQ_HANDLED;
> +}
[snip]
> +static int prpenc_registered(struct v4l2_subdev *sd)
> +{
> + struct prpenc_priv *priv = sd_to_priv(sd);
> + struct imx_media_subdev *imxsd;
> + struct imx_media_pad *pad;
> + int i, ret;
> +
> + /* get media device */
> + priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
> +
> + imxsd = imx_media_find_subdev_by_sd(priv->md, sd);
> + if (IS_ERR(imxsd))
> + return PTR_ERR(imxsd);
> +
> + if (imxsd->num_sink_pads != 1 || imxsd->num_src_pads != 1)
> + return -EINVAL;
> +
> + for (i = 0; i < PRPENC_NUM_PADS; i++) {
> + pad = &imxsd->pad[i];
> + priv->pad[i] = pad->pad;
> + if (priv->pad[i].flags & MEDIA_PAD_FL_SINK)
> + priv->input_pad = i;
> + else
> + priv->output_pad = i;
> +
> + /* set a default mbus format */
> + ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
> + 640, 480, 0, V4L2_FIELD_NONE,
> + &priv->cc[i]);
> + if (ret)
> + return ret;
> + }
> +
> + ret = prpenc_init_controls(priv);
> + if (ret)
> + return ret;
> +
> + ret = media_entity_pads_init(&sd->entity, PRPENC_NUM_PADS, priv->pad);
> + if (ret)
> + goto free_ctrls;
> +
> + return 0;
> +free_ctrls:
> + v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
> + return ret;
if (ret)
v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
return ret;
version is shorter.
> +}
[snip]
> diff --git a/drivers/staging/media/imx/imx-ic-prpvf.c b/drivers/staging/media/imx/imx-ic-prpvf.c
> new file mode 100644
> index 0000000..53ce006
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-ic-prpvf.c
> @@ -0,0 +1,1180 @@
> +/*
> + * V4L2 IC Deinterlacer Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2014-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
Please sort the list of headers alphabetically.
> +#include <media/imx.h>
> +#include "imx-media.h"
> +#include "imx-ic.h"
> +
> +/*
[snip]
> +/* prpvf_out_ch EOF interrupt (progressive frame ready) */
> +static irqreturn_t prpvf_out_eof_interrupt(int irq, void *dev_id)
> +{
> + struct prpvf_priv *priv = dev_id;
> + struct imx_media_dma_buf *done;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->irqlock, flags);
Here spin_lock(&priv->irqlock) should be sufficient.
> +
> + if (priv->last_eof) {
> + complete(&priv->last_eof_comp);
> + priv->last_eof = false;
> + goto unlock;
> + }
> +
> + if (priv->csi_direct) {
> + /* inform CSI of this EOF so it can monitor frame intervals */
> + /* FIXME: frames are coming in twice as fast in direct path! */
> + v4l2_subdev_call(priv->src_sd, core, interrupt_service_routine,
> + 0, NULL);
> + }
> +
> + done = imx_media_dma_buf_get_active(priv->out_ring);
> + /* give the completed buffer to the sink */
> + if (!WARN_ON(!done))
> + imx_media_dma_buf_done(done, IMX_MEDIA_BUF_STATUS_DONE);
> +
> + if (!priv->csi_direct) {
> + /* we're done with the input buffer, queue it back */
> + imx_media_dma_buf_queue(priv->in_ring,
> + priv->curr_in_buf->index);
> +
> + /* current input buffer is now last */
> + priv->last_in_buf = priv->curr_in_buf;
> + } else {
> + /*
> + * priv->next buffer is now the active one due
> + * to IPU double-buffering
> + */
> + imx_media_dma_buf_set_active(priv->next_out_buf);
> + }
> +
> + /* bump the EOF timeout timer */
> + mod_timer(&priv->eof_timeout_timer,
> + jiffies + msecs_to_jiffies(IMX_MEDIA_EOF_TIMEOUT));
> +
> + if (priv->csi_direct) {
> + prepare_prpvf_out_buffer(priv);
> + /* toggle IPU double-buffer index */
> + priv->ipu_buf_num ^= 1;
> + }
> +
> +unlock:
> + spin_unlock_irqrestore(&priv->irqlock, flags);
> + return IRQ_HANDLED;
> +}
> +
[snip]
> diff --git a/drivers/staging/media/imx/imx-ic.h b/drivers/staging/media/imx/imx-ic.h
> new file mode 100644
> index 0000000..9aed5f5
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-ic.h
> @@ -0,0 +1,36 @@
> +/*
> + * V4L2 Image Converter Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#ifndef _IMX_IC_H
> +#define _IMX_IC_H
> +
Please add header files or declarations of all used structs.
> +struct imx_ic_priv {
> + struct device *dev;
> + struct v4l2_subdev sd;
> + int ipu_id;
> + int task_id;
> + void *task_priv;
> +};
> +
> +struct imx_ic_ops {
> + struct v4l2_subdev_ops *subdev_ops;
> + struct v4l2_subdev_internal_ops *internal_ops;
> + struct media_entity_operations *entity_ops;
> +
> + int (*init)(struct imx_ic_priv *ic_priv);
> + void (*remove)(struct imx_ic_priv *ic_priv);
> +};
> +
> +extern struct imx_ic_ops imx_ic_prpenc_ops;
> +extern struct imx_ic_ops imx_ic_prpvf_ops;
> +extern struct imx_ic_ops imx_ic_pp_ops;
> +
> +#endif
> +
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH v4 1/4] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cyrille Pitchen @ 2017-01-04 14:52 UTC (permalink / raw)
To: Cédric Le Goater, Cyrille Pitchen,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Mark Rutland, Boris Brezillon, devicetree-u79uwXL29TY76Z2rM5mHXA,
Richard Weinberger, Marek Vasut, Rob Herring, Joel Stanley,
Brian Norris, David Woodhouse
In-Reply-To: <645db8c4-7f3c-f8bf-ddd9-3f513ce2ed14-Bxea+6Xhats@public.gmane.org>
Hi Cedric,
Le 21/12/2016 à 17:47, Cédric Le Goater a écrit :
> Hello Cyrille,
>
>>>> all aspeed_smc_[read|write]_[reg|user]() functions call
>>>> aspeed_smc_[start|stop]_user(), so this driver always selects the "USER"
>>>> mode and configures the control register based on chip->ctrl_val[smc_base].
>>>
>>> yes.
>>>
>>> Maybe you think it is too early to prepare ground for the other
>>> mode ? Is that really confusing in the code ? Do you think I should
>>> remove it for the initial support in the driver and stick to 'User'
>>> mode.
>>>
>>
>> I think it is not a big deal, at least technically. This is more a
>> psychological aspect of the review: the bigger patches, the more scarier.
>> I mean it requires more time and courage to dig into the source code hence
>> to understand what the driver actually does.
>> Sometime, it's better to split a big patch into many incremental and
>> smaller patches so it's more easy for reviewers to understand each part as
>> an independent feature. It also reveals the driver evolution during the
>> development process hence it helps to understand where it goes and what are
>> the next improvements to come.
>
> I fully agree. It is just that we have been sitting on the code for more
> than a year, using it in production and we should send to mainline
> much sooner. that was a mistake of us ...
>
>> Anyway, since the review is done now, on my side I won't ask you to remove
>> or split the support of the 'Command' mode in a separated patch.
>> I let you do as you want, if it help you to introduce some part of the
>> support of this 'Command' mode now even if not completed yet, no problem on
>> my side :)
>>
>> I was just giving you some pieces of advice for the next time if you want
>> to speed up the review of another patch introducing new features.
>>
>> However, I will just ask you one more version handling the dummy cycles
>> properly as it would help us for the global maintenance of the spi-nor
>> subsystem. This is the only mandatory modification I ask you, after that I
>> think it would be ok for me and since Marek has already reviewed your
>> driver, it would be ready to be merged into the spi-nor tree.
>
> Sending a v5 which should address your comments.
>
> I have removed the label property and will start a new thread in the
> topic. Any hints on which binding we could add this label prop ?
>
Here I will provide just few thoughts about this new DT property. I don't
pretend this is what should be done. I still think other mtd maintainers
should be involved to discuss this topic.
First the DT property name "label" sounds good to me: it is consistent with
"label" DT property used to name mtd partitions. However, I don't think it
should be documented in jedec,spi-nor.txt but *maybe* in partition.txt as
the purpose of this new DT property seems very close to the "label"
property of partition nodes: let's think about some hard-disk device
(/dev/sda) and its partition devices (/dev/sdaX).
Besides, the concept of this memory label is not limited to SPI NOR but
could also apply to NAND memories or any other MTD handled memories.
Hence the DT property might be handled by drivers/mtd/ofpart.c instead of
being handled by spi-nor.c or by each SPI NOR memory controller driver.
Finally, I guess we should take time to discuss and all agree what should
be done precisely before introducing a new DT property because one general
rule with DTB files is that users should be able to update their kernel
image (zImage, uImage, ...) without changing their DTB: device trees should
be backward compatible. Hence if we make a wrong choice today, we are
likely to have to live with it and keep supporting that bad choice.
Anyway, maybe you could describe a little bit your use case; what you
intend to do with this label from you userspace application.
Best regards,
Cyrille
>> Anyway, thanks for taking time to explain us how your driver work :)
>
> Thanks for the review !
>
> C.
>
>
>> Best regards,
>>
>> Cyrille
>>
>>
>>
>>>>> +static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>>>>> + struct device_node *np, struct resource *r)
>>>>> +{
>>>>> + const struct aspeed_smc_info *info = controller->info;
>>>>> + struct device *dev = controller->dev;
>>>>> + struct device_node *child;
>>>>> + unsigned int cs;
>>>>> + int ret = -ENODEV;
>>>>> +
>>>>> + for_each_available_child_of_node(np, child) {
>>>>> + struct aspeed_smc_chip *chip;
>>>>> + struct spi_nor *nor;
>>>>> + struct mtd_info *mtd;
>>>>> +
>>>>> + /* This driver does not support NAND or NOR flash devices. */
>>>>> + if (!of_device_is_compatible(child, "jedec,spi-nor"))
>>>>> + continue;
>>>>> +
>>>>> + ret = of_property_read_u32(child, "reg", &cs);
>>>>> + if (ret) {
>>>>> + dev_err(dev, "Couldn't not read chip select.\n");
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (cs >= info->nce) {
>>>>> + dev_err(dev, "Chip select %d out of range.\n",
>>>>> + cs);
>>>>> + ret = -ERANGE;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (controller->chips[cs]) {
>>>>> + dev_err(dev, "Chip select %d already in use by %s\n",
>>>>> + cs, dev_name(controller->chips[cs]->nor.dev));
>>>>> + ret = -EBUSY;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
>>>>> + if (!chip) {
>>>>> + ret = -ENOMEM;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + chip->controller = controller;
>>>>> + chip->ctl = controller->regs + info->ctl0 + cs * 4;
>>>>> + chip->cs = cs;
>>>>> +
>>>>> + nor = &chip->nor;
>>>>> + mtd = &nor->mtd;
>>>>> +
>>>>> + nor->dev = dev;
>>>>> + nor->priv = chip;
>>>>> + spi_nor_set_flash_node(nor, child);
>>>>> + nor->read = aspeed_smc_read_user;
>>>>> + nor->write = aspeed_smc_write_user;
>>>>> + nor->read_reg = aspeed_smc_read_reg;
>>>>> + nor->write_reg = aspeed_smc_write_reg;
>>>>> + nor->prepare = aspeed_smc_prep;
>>>>> + nor->unprepare = aspeed_smc_unprep;
>>>>> +
>>>>> + mtd->name = of_get_property(child, "label", NULL);
>>>>
>>>> This new "label" DT property should be removed from this patch and send
>>>> in a dedicated patch to be discussed separately. However I agree with
>>>> Marek: it looks generic so maybe it could be managed directly from
>>>> mtd_device_register() since setting such as name could also be done when
>>>> using a NAND flash. Anyway, I don't see the link between this name and
>>>> spi-nor. Hence I don't think the DT property should be documented in
>>>> jedec,spi-nor.txt.
>>>
>>> OK. I will remove it in the next version.
>>>
>>>> Be patient because I expect such a topic to be discussed quite a long
>>>> time before we all agree, even if this is "just" a new DT property ;)
>>>
>>> yeah. I expected that also :) But it is quite pratical to give user
>>> space a hint on the flash nature. Systems can have up to 4 different
>>> ones. So there is need for it IMO.
>>>
>>> How should I proceed then ? Shall I start a discussion with a single
>>> patch changing mtd_device_register() ? but I need to know which binding
>>> would be the more consensual for such a prop.
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>>>
>>>> Best regards,
>>>>
>>>> Cyrille
>>>>
>>>>
>>>>> +
>>>>> + ret = aspeed_smc_chip_setup_init(chip, r);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + /*
>>>>> + * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL
>>>>> + * attach when board support is present as determined
>>>>> + * by of property.
>>>>> + */
>>>>> + ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + ret = aspeed_smc_chip_setup_finish(chip);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + ret = mtd_device_register(mtd, NULL, 0);
>>>>> + if (ret)
>>>>> + break;
>>>>> +
>>>>> + controller->chips[cs] = chip;
>>>>> + }
>>>>> +
>>>>> + if (ret)
>>>>> + aspeed_smc_unregister(controller);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static int aspeed_smc_probe(struct platform_device *pdev)
>>>>> +{
>>>>> + struct device_node *np = pdev->dev.of_node;
>>>>> + struct device *dev = &pdev->dev;
>>>>> + struct aspeed_smc_controller *controller;
>>>>> + const struct of_device_id *match;
>>>>> + const struct aspeed_smc_info *info;
>>>>> + struct resource *res;
>>>>> + int ret;
>>>>> +
>>>>> + match = of_match_device(aspeed_smc_matches, &pdev->dev);
>>>>> + if (!match || !match->data)
>>>>> + return -ENODEV;
>>>>> + info = match->data;
>>>>> +
>>>>> + controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
>>>>> + info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
>>>>> + if (!controller)
>>>>> + return -ENOMEM;
>>>>> + controller->info = info;
>>>>> + controller->dev = dev;
>>>>> +
>>>>> + mutex_init(&controller->mutex);
>>>>> + platform_set_drvdata(pdev, controller);
>>>>> +
>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>>> + controller->regs = devm_ioremap_resource(dev, res);
>>>>> + if (IS_ERR(controller->regs)) {
>>>>> + dev_err(dev, "Cannot remap controller address.\n");
>>>>> + return PTR_ERR(controller->regs);
>>>>> + }
>>>>> +
>>>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>>>> + controller->ahb_base = devm_ioremap_resource(dev, res);
>>>>> + if (IS_ERR(controller->ahb_base)) {
>>>>> + dev_err(dev, "Cannot remap controller address.\n");
>>>>> + return PTR_ERR(controller->ahb_base);
>>>>> + }
>>>>> +
>>>>> + ret = aspeed_smc_setup_flash(controller, np, res);
>>>>> + if (ret)
>>>>> + dev_err(dev, "Aspeed SMC probe failed %d\n", ret);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static struct platform_driver aspeed_smc_driver = {
>>>>> + .probe = aspeed_smc_probe,
>>>>> + .remove = aspeed_smc_remove,
>>>>> + .driver = {
>>>>> + .name = DEVICE_NAME,
>>>>> + .of_match_table = aspeed_smc_matches,
>>>>> + }
>>>>> +};
>>>>> +
>>>>> +module_platform_driver(aspeed_smc_driver);
>>>>> +
>>>>> +MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
>>>>> +MODULE_AUTHOR("Cedric Le Goater <clg-Bxea+6Xhats@public.gmane.org>");
>>>>> +MODULE_LICENSE("GPL v2");
>>>>>
>>>>
>>>
>>>
>>
>
>
--
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] dt-bindings: qman: Remove pool channel node
From: Rob Herring @ 2017-01-04 14:52 UTC (permalink / raw)
To: Madalin Bucur
Cc: mark.rutland-5wv7dgnIgG8, oss-fOR+EgIDQEHk1uMJSBkQmQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1483527829-8369-1-git-send-email-madalin.bucur-3arQi8VN3Tc@public.gmane.org>
On Wed, Jan 04, 2017 at 01:03:49PM +0200, Madalin Bucur wrote:
> From: Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
>
> No device tree has these, nor does any driver look for them.
>
> Signed-off-by: Scott Wood <oss-fOR+EgIDQEHk1uMJSBkQmQ@public.gmane.org>
> ---
> .../devicetree/bindings/soc/fsl/qman-portals.txt | 20 --------------------
> 1 file changed, 20 deletions(-)
Applied, thanks.
Rob
>
> diff --git a/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt b/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> index 47e46cc..5a34f3a 100644
> --- a/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> +++ b/Documentation/devicetree/bindings/soc/fsl/qman-portals.txt
> @@ -5,7 +5,6 @@ Copyright (C) 2008 - 2014 Freescale Semiconductor Inc.
> CONTENTS
>
> - QMan Portal
> - - QMan Pool Channel
> - Example
>
> QMan Portal Node
> @@ -82,25 +81,6 @@ These subnodes should have the following properties:
> Definition: The phandle to the particular hardware device that this
> portal is connected to.
>
> -DPAA QMan Pool Channel Nodes
> -
> -Pool Channels are defined with the following properties.
> -
> -PROPERTIES
> -
> -- compatible
> - Usage: Required
> - Value type: <stringlist>
> - Definition: Must include "fsl,qman-pool-channel"
> - May include "fsl,<SoC>-qman-pool-channel"
> -
> -- fsl,qman-channel-id
> - Usage: Required
> - Value type: <u32>
> - Definition: The hardware index of the channel. This can also be
> - determined by dividing any of the channel's 8 work queue
> - IDs by 8
> -
> EXAMPLE
>
> The example below shows a (P4080) QMan portals container/bus node with two portals
> --
> 2.1.0
>
> --
> 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
--
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: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2017-01-04 14:53 UTC (permalink / raw)
To: Kalle Valo
Cc: Rob Herring,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Rafał Miłecki
In-Reply-To: <874m1ej3oz.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On 4 January 2017 at 15:32, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> On 3 January 2017 at 20:55, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Wed, Dec 28, 2016 at 04:59:54PM +0100, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>
>>>> This new file should be used for properties handled at higher level and
>>>> so usable with all drivers.
>>>
>>> Why is this needed? Where would this data normally come from?
>>
>> Vendors limit choice of channels at their web user interface level. I
>> want to do better and report proper channels directly at kernel level
>> instead of masking them in every possible configuration tool.
>
> Why do vendors limit the channels? Is it because of a hardware
> limitation (antenna does not support, or not calibrated, for a certain
> band etc) or something else?
Please review & comment on the latest version, currently V5:
https://patchwork.kernel.org/patch/9495795/
"This can be used to specify extra hardware limitations caused by e.g.
used antennas or power amplifiers."
--
Rafał
^ permalink raw reply
* Re: [PATCH v6 1/5] dt-bindings: zte: documents zx2967 power domain driver bindings
From: Rob Herring @ 2017-01-04 14:54 UTC (permalink / raw)
To: Baoyou Xie
Cc: jun.nie, mark.rutland, krzk, arnd, ulf.hansson, amitdanielk,
claudiu.manoil, yangbo.lu, pankaj.dubey, geert+renesas,
laurent.pinchart, linux-arm-kernel, devicetree, linux-kernel,
shawnguo, xie.baoyou, chen.chaokai, wang.qiang01
In-Reply-To: <1483530494-14177-1-git-send-email-baoyou.xie@linaro.org>
On Wed, Jan 04, 2017 at 07:48:10PM +0800, Baoyou Xie wrote:
> This patch documents devicetree tree bindings for the ZTE zx2967
> power domain driver.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> ---
> Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt b/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
> new file mode 100644
> index 0000000..1476318
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/zte/pd-2967xx.txt
> @@ -0,0 +1,17 @@
> +* ZTE 2967 SoC Power Domains
> +
> +2967 processors include support for multiple power domains which are used
> +to gate power to one or more peripherals on the processor.
> +
> +Required Properties:
> +- compatible: should be one of the following.
> + * zte,zx296718-pcu - for zx296718 board's power domain.
board? I'd expect this is for an SoC?
> +- reg: physical base address of the controller and length of memory mapped
> + region.
#power-domain-cells needed?
> +
> +Example:
> +
> + pcu_domain: pcu@0x00117000 {
pcu@117000
> + compatible = "zte,zx296718-pcu";
> + reg = <0x00117000 0x1000>;
> + };
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 14/19] media: imx: Add Camera Interface subdev driver
From: Vladimir Zapolskiy @ 2017-01-04 14:55 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-15-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> This is the camera interface driver that provides the v4l2
> user interface. Frames can be received from various sources:
>
> - directly from SMFC for capturing unconverted images directly from
> camera sensors.
>
> - from the IC pre-process encode task.
>
> - from the IC pre-process viewfinder task.
>
> - from the IC post-process task.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/staging/media/imx/Makefile | 2 +-
> drivers/staging/media/imx/imx-camif.c | 1010 +++++++++++++++++++++++++++++++++
> 2 files changed, 1011 insertions(+), 1 deletion(-)
> create mode 100644 drivers/staging/media/imx/imx-camif.c
>
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index d2a962c..fe9e992 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -8,4 +8,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
>
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
> -
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o imx-csi.o imx-smfc.o
as an option.
> diff --git a/drivers/staging/media/imx/imx-camif.c b/drivers/staging/media/imx/imx-camif.c
> new file mode 100644
> index 0000000..3cf167e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-camif.c
> @@ -0,0 +1,1010 @@
> +/*
> + * Video Camera Capture Subdev for Freescale i.MX5/6 SOC
> + *
> + * Copyright (c) 2012-2016 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/fs.h>
> +#include <linux/timer.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/of_platform.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-event.h>
Please sort the list of headers alphabetically.
> +#include <video/imx-ipu-v3.h>
> +#include <media/imx.h>
> +#include "imx-media.h"
> +
> +#define DEVICE_NAME "imx-media-camif"
I would propose to drop this macro.
> +
> +#define CAMIF_NUM_PADS 2
> +
> +#define CAMIF_DQ_TIMEOUT 5000
Add a comment about time unit?
> +
> +struct camif_priv;
> +
This is a leftover apparently.
> +struct camif_priv {
> + struct device *dev;
> + struct video_device vfd;
> + struct media_pipeline mp;
> + struct imx_media_dev *md;
[snip]
> +static int camif_probe(struct platform_device *pdev)
> +{
> + struct imx_media_internal_sd_platformdata *pdata;
> + struct camif_priv *priv;
> + struct video_device *vfd;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, priv);
> + priv->dev = &pdev->dev;
> +
> + pdata = priv->dev->platform_data;
> +
> + mutex_init(&priv->mutex);
> + spin_lock_init(&priv->q_lock);
> +
> + v4l2_subdev_init(&priv->sd, &camif_subdev_ops);
> + v4l2_set_subdevdata(&priv->sd, priv);
> + priv->sd.internal_ops = &camif_internal_ops;
> + priv->sd.entity.ops = &camif_entity_ops;
> + priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->sd.dev = &pdev->dev;
> + priv->sd.owner = THIS_MODULE;
> + /* get our group id and camif id */
> + priv->sd.grp_id = pdata->grp_id;
> + priv->id = (pdata->grp_id >> IMX_MEDIA_GRP_ID_CAMIF_BIT) - 1;
> + strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
> + snprintf(camif_videodev.name, sizeof(camif_videodev.name),
> + "%s devnode", pdata->sd_name);
> +
> + priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
> +
> + vfd = &priv->vfd;
> + *vfd = camif_videodev;
> + vfd->lock = &priv->mutex;
> + vfd->queue = &priv->buffer_queue;
> +
> + video_set_drvdata(vfd, priv);
> +
> + v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
> +
> + ret = v4l2_async_register_subdev(&priv->sd);
> + if (ret)
> + goto free_ctrls;
> +
> + return 0;
> +free_ctrls:
> + v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
> + return ret;
A shorter version:
if (ret)
v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
return ret;
> +}
> +
> +static int camif_remove(struct platform_device *pdev)
> +{
> + struct camif_priv *priv =
> + (struct camif_priv *)platform_get_drvdata(pdev);
> +
> + v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
> + v4l2_async_unregister_subdev(&priv->sd);
> + media_entity_cleanup(&priv->sd.entity);
> + v4l2_device_unregister_subdev(&priv->sd);
> +
> + return 0;
> +}
> +
> +static const struct platform_device_id camif_ids[] = {
> + { .name = DEVICE_NAME },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, camif_ids);
> +
> +static struct platform_driver imx_camif_driver = {
> + .probe = camif_probe,
> + .remove = camif_remove,
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,
Please drop the owner assignment.
> + },
> +};
> +
> +module_platform_driver(imx_camif_driver);
> +
> +MODULE_DESCRIPTION("i.MX camera interface subdev driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH] mailbox: sti: Fix mbox-names copy and paste error
From: Rob Herring @ 2017-01-04 14:57 UTC (permalink / raw)
To: Lee Jones
Cc: patrice.chotard, jassisinghbrar, linux-arm-kernel, linux-kernel,
kernel, devicetree
In-Reply-To: <20170104120527.24806-1-lee.jones@linaro.org>
On Wed, Jan 04, 2017 at 12:05:27PM +0000, Lee Jones wrote:
> Due to an over-sight, mbox-names has become mbox-name in some instances.
>
> Let's put it right.
>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> Documentation/devicetree/bindings/mailbox/sti-mailbox.txt | 4 ++--
> arch/arm/boot/dts/stih407-family.dtsi | 8 ++++----
> drivers/mailbox/mailbox-sti.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> index 351f612..648d176 100644
> --- a/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> +++ b/Documentation/devicetree/bindings/mailbox/sti-mailbox.txt
> @@ -9,7 +9,7 @@ Controller
> Required properties:
> - compatible : Should be "st,stih407-mailbox"
> - reg : Offset and length of the device's register set
> -- mbox-name : Name of the mailbox
> +- mbox-names : Name of the mailbox
It's worse than this. mbox-names is for the mailbox client side. This
should just be dropped. Plus, *-names is pointless when there is only 1
element.
Rob
^ permalink raw reply
* Re: [PATCH v2 15/19] media: imx: Add MIPI CSI-2 Receiver subdev driver
From: Vladimir Zapolskiy @ 2017-01-04 15:05 UTC (permalink / raw)
To: Steve Longerbeam, shawnguo, kernel, fabio.estevam, robh+dt,
mark.rutland, linux, mchehab, gregkh, p.zabel
Cc: devel, devicetree, Steve Longerbeam, linux-kernel,
linux-arm-kernel, linux-media
In-Reply-To: <1483477049-19056-16-git-send-email-steve_longerbeam@mentor.com>
On 01/03/2017 10:57 PM, Steve Longerbeam wrote:
> Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
> for sensors with a MIPI CSI2 interface.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> drivers/staging/media/imx/Makefile | 1 +
> drivers/staging/media/imx/imx-mipi-csi2.c | 509 ++++++++++++++++++++++++++++++
> 2 files changed, 510 insertions(+)
> create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
>
> diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
> index fe9e992..0decef7 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
> obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
> +obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
> diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c b/drivers/staging/media/imx/imx-mipi-csi2.c
> new file mode 100644
> index 0000000..84df16e
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-mipi-csi2.c
> @@ -0,0 +1,509 @@
> +/*
> + * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
> + *
> + * Copyright (c) 2012-2014 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/types.h>
> +#include <linux/init.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/list.h>
> +#include <linux/irq.h>
> +#include <linux/of_device.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/v4l2-async.h>
Please sort the list of headers alphabetically.
> +#include <asm/mach/irq.h>
Why do you need to include this header?
> +#include <video/imx-ipu-v3.h>
> +#include "imx-media.h"
> +
[snip]
> +static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> +{
> + struct imxcsi2_dev *csi2 = sd_to_dev(sd);
> + int i, ret = 0;
> +
> + if (!csi2->src_sd)
> + return -EPIPE;
> + for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
> + if (csi2->sink_sd[i])
> + break;
> + }
> + if (i >= CSI2_NUM_SRC_PADS)
> + return -EPIPE;
> +
> + v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
> +
> + if (enable && !csi2->stream_on) {
> + clk_prepare_enable(csi2->pix_clk);
It can complicate the design for you, but in general clk_prepare_enable()
can return an error.
> + ret = imxcsi2_dphy_wait(csi2);
> + if (ret)
> + clk_disable_unprepare(csi2->pix_clk);
> + } else if (!enable && csi2->stream_on) {
> + clk_disable_unprepare(csi2->pix_clk);
> + }
> +
> + if (!ret)
> + csi2->stream_on = enable;
> + return ret;
> +}
> +
[snip]
> +
> +static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
> +{
> + struct device_node *node = csi2->dev->of_node;
> + struct device_node *epnode;
> + struct v4l2_of_endpoint ep;
> + int ret = 0;
> +
> + epnode = of_graph_get_next_endpoint(node, NULL);
> + if (!epnode) {
> + v4l2_err(&csi2->sd, "failed to get endpoint node\n");
> + return -EINVAL;
> + }
> +
> + v4l2_of_parse_endpoint(epnode, &ep);
Do of_node_put(epnode) here and remove 'out' goto label.
> + if (ep.bus_type != V4L2_MBUS_CSI2) {
> + v4l2_err(&csi2->sd, "invalid bus type, must be MIPI CSI2\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + csi2->bus = ep.bus.mipi_csi2;
> +
> + v4l2_info(&csi2->sd, "data lanes: %d\n", csi2->bus.num_data_lanes);
> + v4l2_info(&csi2->sd, "flags: 0x%08x\n", csi2->bus.flags);
> +out:
> + of_node_put(epnode);
> + return ret;
> +}
> +
[snip]
> +static const struct of_device_id imxcsi2_dt_ids[] = {
> + { .compatible = "fsl,imx-mipi-csi2", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
> +
> +static struct platform_driver imxcsi2_driver = {
> + .driver = {
> + .name = DEVICE_NAME,
> + .owner = THIS_MODULE,
Please drop .owner assignment.
> + .of_match_table = imxcsi2_dt_ids,
> + },
> + .probe = imxcsi2_probe,
> + .remove = imxcsi2_remove,
> +};
> +
> +module_platform_driver(imxcsi2_driver);
> +
> +MODULE_DESCRIPTION("i.MX5/6 MIPI CSI-2 Receiver driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
> +
>
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH V2 1/5] Documetation: samsung-phy: add the exynos-pcie-phy binding
From: Rob Herring @ 2017-01-04 15:17 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, mark.rutland-5wv7dgnIgG8,
kgene-DgEjT+Ai2ygdnm+yROfE0A, krzk-DgEjT+Ai2ygdnm+yROfE0A,
kishon-l0cyMroinI0, jingoohan1-Re5JQEeQqe8AvxtiuMwx3w,
vivek.gautam-sgV2jX0FEOL9JmXXK+q4OQ,
pankaj.dubey-Sze3O3UU22JBDgjK7y7TUQ,
alim.akhtar-Sze3O3UU22JBDgjK7y7TUQ, cpgs-Sze3O3UU22JBDgjK7y7TUQ
In-Reply-To: <20170104123435.30740-2-jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
On Wed, Jan 04, 2017 at 09:34:31PM +0900, Jaehoon Chung wrote:
> Adds the exynos-pcie-phy binding for Exynos PCIe PHY.
> This is for using generic PHY framework.
>
> Signed-off-by: Jaehoon Chung <jh80.chung-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
> Changelog on V2:
> - Remove the child node.
> - Add 2nd address to the parent reg prop.
>
> Documentation/devicetree/bindings/phy/samsung-phy.txt | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
Acked-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 V2 3/5] Documetation: binding: modify the exynos5440 pcie binding
From: Rob Herring @ 2017-01-04 15:18 UTC (permalink / raw)
To: Jaehoon Chung
Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
mark.rutland, kgene, krzk, kishon, jingoohan1, vivek.gautam,
pankaj.dubey, alim.akhtar, cpgs
In-Reply-To: <20170104123435.30740-4-jh80.chung@samsung.com>
On Wed, Jan 04, 2017 at 09:34:33PM +0900, Jaehoon Chung wrote:
> According to using PHY framework, updates the exynos5440-pcie binding.
> For maintaining backward compatibility, leaves the current dt-binding.
> (It should be deprecated.)
>
> Recommends to use the Phy Framework and "config" property to follow
> the designware-pcie binding.
> If you use the old way, can see "mssing *config* reg space" message.
> Because the getting configuration space address from range is old way.
>
> NOTE: When use the "config" property, first name of 'reg-names' must be
> set to "elbi". Otherwise driver can't maintain the backward capability.
>
> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
> ---
> Changelog on V2:
> - Describes more commit message
> - Fixes the typos
> - Adds the new example for using PHY framework
> - Deprecated the old dt-binding description
> - Removes 'phy-names'
>
> .../bindings/pci/samsung,exynos5440-pcie.txt | 29 ++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Rob Herring @ 2017-01-04 15:26 UTC (permalink / raw)
To: Kedareswara rao Appana
Cc: mark.rutland-5wv7dgnIgG8, dan.j.williams-ral2JQCrhuEAvxtiuMwx3w,
vinod.koul-ral2JQCrhuEAvxtiuMwx3w,
michal.simek-gjFFaj9aHVfQT0dZR+AlfA,
soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA,
appanad-gjFFaj9aHVfQT0dZR+AlfA,
moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
luis-HiykPkW1eAzzDCI4PIEvbQC/G2K4zDHf,
Jose.Abreu-HKixBCOQz3hWk0Htik3J/w,
dmaengine-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1483536954-27203-3-git-send-email-appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
On Wed, Jan 04, 2017 at 07:05:53PM +0530, Kedareswara rao Appana wrote:
> When VDMA is configured for more than one frame in the h/w
> for example h/w is configured for n number of frames and user
> Submits n number of frames and triggered the DMA using issue_pending API.
> In the current driver flow we are submitting one frame at a time
> but we should submit all the n number of frames at one time as the h/w
> Is configured for n number of frames.
Please fix run-on sentences, capitalization, and word wrapping.
>
> This patch fixes this issue.
>
> Reviewed-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Kedareswara rao Appana <appanad-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> Changes for v4:
> ---> Add Check for framestore configuration on Transmit case as well
> as suggested by Jose Abreu.
> ---> Modified the dev_dbg checks to dev_warn checks as suggested
> by Jose Abreu.
> Changes for v3:
> ---> Added Checks for frame store configuration. If frame store
> Configuration is not present at the h/w level and user
> Submits less frames added debug prints in the driver as relevant.
> Changes for v2:
> ---> Fixed race conditions in the driver as suggested by Jose Abreu
> ---> Fixed unnecessray if else checks in the vdma_start_transfer
> as suggested by Laurent Pinchart.
>
> .../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
> drivers/dma/xilinx/xilinx_dma.c | 79 +++++++++++++++-------
> 2 files changed, 58 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> index a2b8bfa..1f65e09 100644
> --- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
> @@ -66,6 +66,8 @@ Optional child node properties:
> Optional child node properties for VDMA:
> - xlnx,genlock-mode: Tells Genlock synchronization is
> enabled/disabled in hardware.
> +- xlnx,fstore-config: Tells Whether Frame Store Configuration is
> + enabled/disabled in hardware.
What's the default (when not present)? That should be the most
common case. Looks like the code treats this as bool, but that's
not clear here. The name is not clear what it is doing. Enabling or
disabling the feature?
> Optional child node properties for AXI DMA:
> -dma-channels: Number of dma channels in child node.
>
--
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 v2 05/19] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
From: Fabio Estevam @ 2017-01-04 15:26 UTC (permalink / raw)
To: Steve Longerbeam
Cc: Mark Rutland, devel, Steve Longerbeam, Philipp Zabel,
devicetree@vger.kernel.org, Greg Kroah-Hartman,
Russell King - ARM Linux, linux-kernel, robh+dt@kernel.org,
Sascha Hauer, Fabio Estevam, mchehab, Shawn Guo,
linux-arm-kernel@lists.infradead.org, linux-media
In-Reply-To: <1483477049-19056-6-git-send-email-steve_longerbeam@mentor.com>
On Tue, Jan 3, 2017 at 6:57 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> + camera: ov5642@3c {
> + compatible = "ovti,ov5642";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5642>;
> + clocks = <&clks IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + reg = <0x3c>;
> + xclk = <24000000>;
> + DOVDD-supply = <&vgen4_reg>; /* 1.8v */
> + AVDD-supply = <&vgen5_reg>; /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
Please use vgen3 so that by default we have the valid AVDD-supply for
revC boards which is more recent and more the users have access to.
> + mipi_camera: ov5640@3c {
> + compatible = "ovti,ov5640_mipi";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ov5640>;
> + reg = <0x3c>;
> + clocks = <&clks IMX6QDL_CLK_CKO>;
> + clock-names = "xclk";
> + xclk = <24000000>;
> + DOVDD-supply = <&vgen4_reg>; /* 1.8v */
> + AVDD-supply = <&vgen5_reg>; /* 2.8v, rev C board is VGEN3
> + rev B board is VGEN5 */
Same here.
> + pinctrl_ov5640: ov5640grp {
> + fsl,pins = <
> + MX6QDL_PAD_SD1_DAT2__GPIO1_IO19 0x80000000
> + MX6QDL_PAD_SD1_CLK__GPIO1_IO20 0x80000000
Please avoid all the 0x80000000 IOMUX settings and replace them by
their real values.
^ permalink raw reply
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