* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Ramiro Oliveira @ 2016-12-12 12:15 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <20161212114900.GS16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hi Sakari
On 12/12/2016 11:49 AM, Sakari Ailus wrote:
> Hi Ramiro,
>
> On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
>> Hi Sakari,
>>
>> Thank you for the feedback.
>>
>> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
>>> Hi Ramiro,
>>>
>>> Thank you for the patch.
>>>
>>> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
>>>> Add device tree documentation.
>>>>
>>>> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
>>>> 1 file changed, 19 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> new file mode 100644
>>>> index 0000000..4c91b3b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>>> @@ -0,0 +1,19 @@
>>>> +Omnivision OV5647 raw image sensor
>>>> +---------------------------------
>>>> +
>>>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
>>>> +and CCI (I2C compatible) control bus.
>>>> +
>>>> +Required properties:
>>>> +
>>>> +- compatible : "ovti,ov5647";
>>>> +- reg : I2C slave address of the sensor;
>>>> +
>>>> +The common video interfaces bindings (see video-interfaces.txt) should be
>>>> +used to specify link to the image data receiver. The OV5647 device
>>>> +node should contain one 'port' child node with an 'endpoint' subnode.
>>>> +
>>>> +Following properties are valid for the endpoint node:
>>>> +
>>>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>>>> + video-interfaces.txt. The sensor supports only two data lanes.
>>>
>>> Doesn't this sensor require a external clock, a reset GPIO and / or a
>>> regulator or a few? Do you need data-lanes, unless you can change the order
>>> or the number?
>>
>> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
>> external clock but it's fixed and not controlled by SW. Should I add a property
>> for this?
>
> The sensor datasheet defines a power-up and power-down sequence for the
> device. If you don't implement these sequences in the driver on a DT based
> system, nothing suggests that they're implemented correctly. Could it be
> that the boot loader simply enables the regulators or another device
> requires them to be enabled?
>
> I presume at least the reset GPIO should be controlled explicitly in order
> to ensure correct function. Although hardware can be surprising: I have one
> production system that has no reset GPIO for the sensor albeit the sensor
> datasheet says that's part of the power up sequence.
>
Sorry for the misunderstanding. I wanted to say that, there is no SW controlled
reset. In the board we're using to connect the sensor to our D-PHY we have a
GPIO controller that when it receives power, it removes the sensor from reset,
so I have no control over that.
Regarding the clock, should I create a new property?
And also, regarding the data-lanes, AFAIK it isn't possible to change the order
of the data and clock lanes so should I remove that property?
>>
>>>
>>> An example DT snippet wouldn't hurt.
>>
>> Sure, I can add a example snippet.
>>
>>>
>>
>
--
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 v5 2/2] Add support for OV5647 sensor
From: Sakari Ailus @ 2016-12-12 11:54 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <936d4f19-9a1a-0873-121e-b9471449f70d-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
On Mon, Dec 12, 2016 at 11:39:13AM +0000, Ramiro Oliveira wrote:
> Hi Sakari
>
> On 12/7/2016 11:01 PM, Sakari Ailus wrote:
> > Hi Ramiro,
> >
> > On Mon, Dec 05, 2016 at 05:36:34PM +0000, Ramiro Oliveira wrote:
> >> Add support for OV5647 sensor.
> >>
> >> Modes supported:
> >> - 640x480 RAW 8
> >>
> >> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> ---
> >> MAINTAINERS | 7 +
> >> drivers/media/i2c/Kconfig | 12 +
> >> drivers/media/i2c/Makefile | 1 +
> >> drivers/media/i2c/ov5647.c | 866 +++++++++++++++++++++++++++++++++++++++++++++
> >> 4 files changed, 886 insertions(+)
> >> create mode 100644 drivers/media/i2c/ov5647.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 52cc077..72e828a 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -8923,6 +8923,13 @@ M: Harald Welte <laforge-TgoAw6mPHtdg9hUCZPvPmw@public.gmane.org>
> >> S: Maintained
> >> F: drivers/char/pcmcia/cm4040_cs.*
> >>
> >> +OMNIVISION OV5647 SENSOR DRIVER
> >> +M: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> +L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> +T: git git://linuxtv.org/media_tree.git
> >> +S: Maintained
> >> +F: drivers/media/i2c/ov5647.c
> >> +
> >> OMNIVISION OV7670 SENSOR DRIVER
> >> M: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> >> L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> >> index b31fa6f..c1b78e5 100644
> >> --- a/drivers/media/i2c/Kconfig
> >> +++ b/drivers/media/i2c/Kconfig
> >> @@ -531,6 +531,18 @@ config VIDEO_OV2659
> >> To compile this driver as a module, choose M here: the
> >> module will be called ov2659.
> >>
> >> +config VIDEO_OV5647
> >> + tristate "OmniVision OV5647 sensor support"
> >> + depends on OF
> >
> > How does this driver depend on OF, other than matching the compatible
> > string?
> >
>
> It doesn't, should I proceed diferently?
You should drop the dependency if you don't need it. But I bet you will
actually need it.
>
> >> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> >> + depends on MEDIA_CAMERA_SUPPORT
> >> + ---help---
> >> + This is a Video4Linux2 sensor-level driver for the OmniVision
> >> + OV5647 camera.
> >> +
> >> + To compile this driver as a module, choose M here: the
> >> + module will be called ov5647.
> >> +
> >> config VIDEO_OV7640
> >> tristate "OmniVision OV7640 sensor support"
> >> depends on I2C && VIDEO_V4L2
> >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> >> index 92773b2..0d9014c 100644
> >> --- a/drivers/media/i2c/Makefile
> >> +++ b/drivers/media/i2c/Makefile
> >> @@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
> >> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
> >> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
> >> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> >> +obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
> >> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> >> new file mode 100644
> >> index 0000000..2aae806
> >> --- /dev/null
> >> +++ b/drivers/media/i2c/ov5647.c
> >> @@ -0,0 +1,866 @@
> >> +/*
> >> + * A V4L2 driver for OmniVision OV5647 cameras.
> >> + *
> >> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
> >> + * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >> + *
> >> + * Based on Omnivision OV7670 Camera Driver
> >> + * Copyright (C) 2006-7 Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> >> + *
> >> + * Copyright (C) 2016, Synopsys, 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 version 2.
> >> + *
> >> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
> >> + * kind, whether express or implied; without even the implied warranty
> >> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + */
> >> +#include <linux/init.h>
> >> +#include <linux/module.h>
> >> +#include <linux/slab.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/delay.h>
> >> +#include <linux/videodev2.h>
> >> +#include <media/v4l2-device.h>
> >> +#include <media/v4l2-ctrls.h>
> >> +#include <media/v4l2-mediabus.h>
> >> +#include <media/v4l2-image-sizes.h>
> >> +#include <media/v4l2-of.h>
> >> +#include <linux/io.h>
> >
> > Alphabetical order, please.
> >
> >> +
> >> +#define SENSOR_NAME "ov5647"
> >> +
> >> +#define OV5647_SW_RESET 0x1003
> >> +#define OV5647_REG_CHIPID_H 0x300A
> >> +#define OV5647_REG_CHIPID_L 0x300B
> >> +
> >> +#define REG_TERM 0xfffe
> >> +#define VAL_TERM 0xfe
> >> +#define REG_DLY 0xffff
> >> +
> >> +#define OV5647_ROW_START 0x01
> >> +#define OV5647_ROW_START_MIN 0
> >> +#define OV5647_ROW_START_MAX 2004
> >> +#define OV5647_ROW_START_DEF 54
> >> +
> >> +#define OV5647_COLUMN_START 0x02
> >> +#define OV5647_COLUMN_START_MIN 0
> >> +#define OV5647_COLUMN_START_MAX 2750
> >> +#define OV5647_COLUMN_START_DEF 16
> >> +
> >> +#define OV5647_WINDOW_HEIGHT 0x03
> >> +#define OV5647_WINDOW_HEIGHT_MIN 2
> >> +#define OV5647_WINDOW_HEIGHT_MAX 2006
> >> +#define OV5647_WINDOW_HEIGHT_DEF 1944
> >> +
> >> +#define OV5647_WINDOW_WIDTH 0x04
> >> +#define OV5647_WINDOW_WIDTH_MIN 2
> >> +#define OV5647_WINDOW_WIDTH_MAX 2752
> >> +#define OV5647_WINDOW_WIDTH_DEF 2592
> >> +
> >> +struct regval_list {
> >> + u16 addr;
> >> + u8 data;
> >> +};
> >> +
> >> +struct cfg_array {
> >> + struct regval_list *regs;
> >> + int size;
> >> +};
> >> +
> >> +struct sensor_win_size {
> >> + int width;
> >> + int height;
> >> + unsigned int hoffset;
> >> + unsigned int voffset;
> >> + unsigned int hts;
> >> + unsigned int vts;
> >> + unsigned int pclk;
> >> + unsigned int mipi_bps;
> >> + unsigned int fps_fixed;
> >> + unsigned int bin_factor;
> >> + unsigned int intg_min;
> >> + unsigned int intg_max;
> >> + void *regs;
> >> + int regs_size;
> >> + int (*set_size)(struct v4l2_subdev *sd);
> >> +};
> >> +
> >> +
> >> +struct ov5647 {
> >> + struct device *dev;
> >> + struct v4l2_subdev sd;
> >> + struct media_pad pad;
> >> + struct mutex lock;
> >> + struct v4l2_mbus_framefmt format;
> >> + struct sensor_format_struct *fmt;
> >> + unsigned int width;
> >> + unsigned int height;
> >> + unsigned int capture_mode;
> >> + int hue;
> >
> > At least capture_mode and hue are unused. Please remove unused fields.
> >
> >> + struct v4l2_fract tpf;
> >> + struct sensor_win_size *current_wins;
> >> +};
> >> +
> >> +static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
> >> +{
> >> + return container_of(sd, struct ov5647, sd);
> >> +}
> >> +
> >> +static struct regval_list sensor_oe_disable_regs[] = {
> >> + {0x3000, 0x00},
> >> + {0x3001, 0x00},
> >> + {0x3002, 0x00},
> >> +};
> >> +
> >> +static struct regval_list sensor_oe_enable_regs[] = {
> >> + {0x3000, 0x0f},
> >> + {0x3001, 0xff},
> >> + {0x3002, 0xe4},
> >> +};
> >> +
> >> +static struct regval_list ov5647_640x480[] = {
> >
> > Does this list expect a certain external clock frequency? If it does, should
> > you check that the actual frequency matches with the expectation?
> >
>
> Yes. But like I said in the DT patch the external clock has a fixed frequency. I
> can add a check if you thinks it's better.
>
> >> + {0x0100, 0x00},
> >> + {0x0103, 0x01},
> >> + {0x3034, 0x08},
> >> + {0x3035, 0x21},
> >> + {0x3036, 0x46},
> >> + {0x303c, 0x11},
> >> + {0x3106, 0xf5},
> >> + {0x3821, 0x07},
> >> + {0x3820, 0x41},
> >> + {0x3827, 0xec},
> >> + {0x370c, 0x0f},
> >> + {0x3612, 0x59},
> >> + {0x3618, 0x00},
> >> + {0x5000, 0x06},
> >> + {0x5001, 0x01},
> >> + {0x5002, 0x41},
> >> + {0x5003, 0x08},
> >> + {0x5a00, 0x08},
> >> + {0x3000, 0x00},
> >> + {0x3001, 0x00},
> >> + {0x3002, 0x00},
> >> + {0x3016, 0x08},
> >> + {0x3017, 0xe0},
> >> + {0x3018, 0x44},
> >> + {0x301c, 0xf8},
> >> + {0x301d, 0xf0},
> >> + {0x3a18, 0x00},
> >> + {0x3a19, 0xf8},
> >> + {0x3c01, 0x80},
> >> + {0x3b07, 0x0c},
> >> + {0x380c, 0x07},
> >> + {0x380d, 0x68},
> >> + {0x380e, 0x03},
> >> + {0x380f, 0xd8},
> >> + {0x3814, 0x31},
> >> + {0x3815, 0x31},
> >> + {0x3708, 0x64},
> >> + {0x3709, 0x52},
> >> + {0x3808, 0x02},
> >> + {0x3809, 0x80},
> >> + {0x380a, 0x01},
> >> + {0x380b, 0xE0},
> >> + {0x3801, 0x00},
> >> + {0x3802, 0x00},
> >> + {0x3803, 0x00},
> >> + {0x3804, 0x0a},
> >> + {0x3805, 0x3f},
> >> + {0x3806, 0x07},
> >> + {0x3807, 0xa1},
> >> + {0x3811, 0x08},
> >> + {0x3813, 0x02},
> >> + {0x3630, 0x2e},
> >> + {0x3632, 0xe2},
> >> + {0x3633, 0x23},
> >> + {0x3634, 0x44},
> >> + {0x3636, 0x06},
> >> + {0x3620, 0x64},
> >> + {0x3621, 0xe0},
> >> + {0x3600, 0x37},
> >> + {0x3704, 0xa0},
> >> + {0x3703, 0x5a},
> >> + {0x3715, 0x78},
> >> + {0x3717, 0x01},
> >> + {0x3731, 0x02},
> >> + {0x370b, 0x60},
> >> + {0x3705, 0x1a},
> >> + {0x3f05, 0x02},
> >> + {0x3f06, 0x10},
> >> + {0x3f01, 0x0a},
> >> + {0x3a08, 0x01},
> >> + {0x3a09, 0x27},
> >> + {0x3a0a, 0x00},
> >> + {0x3a0b, 0xf6},
> >> + {0x3a0d, 0x04},
> >> + {0x3a0e, 0x03},
> >> + {0x3a0f, 0x58},
> >> + {0x3a10, 0x50},
> >> + {0x3a1b, 0x58},
> >> + {0x3a1e, 0x50},
> >> + {0x3a11, 0x60},
> >> + {0x3a1f, 0x28},
> >> + {0x4001, 0x02},
> >> + {0x4004, 0x02},
> >> + {0x4000, 0x09},
> >> + {0x4837, 0x24},
> >> + {0x4050, 0x6e},
> >> + {0x4051, 0x8f},
> >> + {0x0100, 0x01},
> >> +};
> >> +
> >> +struct sensor_format_struct;
> >> +
> >> +/**
> >> + * @short I2C Write operation
> >> + * @param[in] i2c_client I2C client
> >> + * @param[in] reg register address
> >> + * @param[in] val value to write
> >> + * @return Error code
> >> + */
> >> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
> >> +{
> >> + int ret;
> >> + unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = i2c_master_send(client, data, 3);
> >> + if (ret != 3) {
> >> + dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short I2C Read operation
> >> + * @param[in] i2c_client I2C client
> >> + * @param[in] reg register address
> >> + * @param[out] val value read
> >> + * @return Error code
> >> + */
> >> +static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
> >> +{
> >> + int ret;
> >> + unsigned char data_w[2] = { reg >> 8, reg & 0xff };
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> +
> >> + ret = i2c_master_send(client, data_w, 2);
> >> +
> >> + if (ret < 2) {
> >> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> +
> >> +
> >> + ret = i2c_master_recv(client, val, 1);
> >> +
> >> + if (ret < 1) {
> >> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
> >> + __func__, reg);
> >> + return ret < 0 ? ret : -EIO;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static int ov5647_write_array(struct v4l2_subdev *sd,
> >> + struct regval_list *regs, int array_size)
> >> +{
> >> + int i = 0;
> >> + int ret = 0;
> >> +
> >> + if (!regs)
> >> + return -EINVAL;
> >> +
> >> + while (i < array_size) {
> >> + ret = ov5647_write(sd, regs->addr, regs->data);
> >> + if (ret < 0)
> >> + return ret;
> >> + i++;
> >> + regs++;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
> >> +{
> >> + u8 channel_id;
> >> +
> >> + ov5647_read(sd, 0x4814, &channel_id);
> >> + channel_id &= ~(3 << 6);
> >> + ov5647_write(sd, 0x4814, channel_id | (channel << 6));
> >> +}
> >> +
> >> +void ov5647_stream_on(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ov5647_write(sd, 0x4202, 0x00);
> >> + dev_dbg(&client->dev, "Stream on");
> >> + ov5647_write(sd, 0x300D, 0x00);
> >> +}
> >> +
> >> +void ov5647_stream_off(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ov5647_write(sd, 0x4202, 0x0f);
> >> + dev_dbg(&client->dev, "Stream off");
> >> + ov5647_write(sd, 0x300D, 0x01);
> >> +}
> >> +
> >> +/****************************************************************************/
> >> +
> >> +/**
> >> + * @short Set SW standby
> >> + * @param[in] sd v4l2 sd
> >> + * @param[in] stanby standby mode status (on or off)
> >> + * @return Error code
> >> + */
> >> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
> >> +{
> >> + int ret;
> >> + unsigned char rdval;
> >> +
> >> + ret = ov5647_read(sd, 0x0100, &rdval);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + if (standby)
> >> + rdval &= 0xfe;
> >> + else
> >> + rdval |= 0x01;
> >> +
> >> + ret = ov5647_write(sd, 0x0100, rdval);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Store information about the video data format.
> >> + */
> >> +static struct sensor_format_struct {
> >> + u8 *desc;
> >> + u32 mbus_code;
> >> + struct regval_list *regs;
> >> + int regs_size;
> >> + int bpp;
> >
> > At least desc and bpp are unused.
> >
> >> +} sensor_formats[] = {
> >> + {
> >> + .desc = "Raw RGB Bayer",
> >> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
> >> + .regs = ov5647_640x480,
> >> + .regs_size = ARRAY_SIZE(ov5647_640x480),
> >> + .bpp = 1
> >> + },
> >> +};
> >> +#define N_FMTS ARRAY_SIZE(sensor_formats)
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Initialize sensor
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] val not used
> >> + * @return Error code
> >> + */
> >> +static int __sensor_init(struct v4l2_subdev *sd)
> >> +{
> >> + int ret;
> >> + u8 resetval;
> >> + u8 rdval;
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + dev_dbg(&client->dev, "sensor init\n");
> >> +
> >> + ret = ov5647_read(sd, 0x0100, &rdval);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + ov5647_write(sd, 0x4800, 0x25);
> >> + ov5647_stream_off(sd);
> >> +
> >> + ret = ov5647_write_array(sd, ov5647_640x480,
> >> + ARRAY_SIZE(ov5647_640x480));
> >> + if (ret < 0) {
> >> + dev_err(&client->dev, "write sensor_default_regs error\n");
> >> + return ret;
> >> + }
> >> +
> >> + ov5647_set_virtual_channel(sd, 0);
> >> +
> >> + ov5647_read(sd, 0x0100, &resetval);
> >> + if (!(resetval & 0x01)) {
> >> + dev_err(&client->dev, "Device was in SW standby");
> >> + ov5647_write(sd, 0x0100, 0x01);
> >> + }
> >> +
> >> + ov5647_write(sd, 0x4800, 0x04);
> >> + ov5647_stream_on(sd);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Control sensor power state
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] on Sensor power
> >> + * @return Error code
> >> + */
> >> +static int sensor_power(struct v4l2_subdev *sd, int on)
> >> +{
> >> + int ret;
> >> + struct ov5647 *ov5647 = to_state(sd);
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = 0;
> >> + mutex_lock(&ov5647->lock);
> >> +
> >> + if (on) {
> >> + dev_dbg(&client->dev, "OV5647 power on\n");
> >> +
> >> + ret = ov5647_write_array(sd, sensor_oe_enable_regs,
> >> + ARRAY_SIZE(sensor_oe_enable_regs));
> >> +
> >> + ret = __sensor_init(sd);
> >> +
> >> + if (ret < 0)
> >> + dev_err(&client->dev,
> >> + "Camera not available, check Power\n");
> >> + } else {
> >> + dev_dbg(&client->dev, "OV5647 power off\n");
> >> +
> >> + dev_dbg(&client->dev, "disable oe\n");
> >> + ret = ov5647_write_array(sd, sensor_oe_disable_regs,
> >> + ARRAY_SIZE(sensor_oe_disable_regs));
> >> +
> >> + if (ret < 0)
> >> + dev_dbg(&client->dev, "disable oe failed\n");
> >> +
> >> + ret = set_sw_standby(sd, true);
> >> +
> >> + if (ret < 0)
> >> + dev_dbg(&client->dev, "soft stby failed\n");
> >> +
> >> + }
> >> +
> >> + mutex_unlock(&ov5647->lock);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> +/**
> >> + * @short Get register value
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] reg register struct
> >> + * @return Error code
> >> + */
> >> +static int sensor_get_register(struct v4l2_subdev *sd,
> >> + struct v4l2_dbg_register *reg)
> >> +{
> >> + unsigned char val = 0;
> >> + int ret;
> >> +
> >> + ret = ov5647_read(sd, reg->reg & 0xff, &val);
> >> + if (ret != 0)
> >> + return ret;
> >> +
> >> + reg->val = val;
> >> + reg->size = 1;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Set register value
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] reg register struct
> >> + * @return Error code
> >> + */
> >> +static int sensor_set_register(struct v4l2_subdev *sd,
> >> + const struct v4l2_dbg_register *reg)
> >> +{
> >> + return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
> >> +}
> >> +#endif
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Subdev core operations registration
> >> + */
> >> +static const struct v4l2_subdev_core_ops sensor_core_ops = {
> >> + .s_power = sensor_power,
> >
> > The s_power() op will be called by the bridge (ISP) driver as well. You
> > should expect that it may be enabled more than once before being disabled
> > equal number of times.
> >
>
> Should I add an IF check to verify if the sensor is powered on before running
> the power on/off routine.
You need a counter. One way to do that is in the driver itself, or you can
do what I recently did in the smiapp driver, using runtime PM so you don't
explicitly need to manage it:
<URL:https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=smiapp-pm&id=c29df33f9ec94226eab8ee92d8c66ab83c76659a>
Or, before the runtime PM conversion:
<URL:https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media/i2c/smiapp/smiapp-core.c?h=smiapp-pm&id=c8d2bc9bc39ebea8437fd974fdbc21847bb897a3>
The use of runtime PM in sub-device drivers is new and I'm not entirely
happy how it's implemented in the smiapp driver right now, hence it might be
better not to use it (yet). Up to you.
>
> >> +#ifdef CONFIG_VIDEO_ADV_DEBUG
> >> + .g_register = sensor_get_register,
> >> + .s_register = sensor_set_register,
> >> +#endif
> >> +};
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +
> >> +
> >> +/**
> >> + * @short Enumerate available image formats
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] index index
> >> + * @param[in] code MBUS Pixel code
> >> + * @return Error code
> >> + */
> >> +static int sensor_enum_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_mbus_code_enum *code)
> >> +{
> >> + if (code->pad || code->index >= N_FMTS)
> >> + return -EINVAL;
> >> +
> >> + code->code = sensor_formats[code->index].mbus_code;
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Try frame format internal function
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fmt frame format
> >> + * @return Error code
> >> + */
> >> +static int sensor_try_fmt_internal(struct v4l2_subdev *sd,
> >> + struct v4l2_mbus_framefmt *fmt, struct sensor_format_struct **ret_fmt,
> >> + struct sensor_win_size **ret_wsize)
> >> +{
> >> + int index;
> >> +
> >> + for (index = 0; index < N_FMTS; index++)
> >> + if (sensor_formats[index].mbus_code == fmt->code)
> >> + break;
> >> +
> >> + if (index >= N_FMTS)
> >> + return -EINVAL;
> >> +
> >> + if (ret_fmt != NULL)
> >> + *ret_fmt = sensor_formats + index;
> >> +
> >> + fmt->field = V4L2_FIELD_NONE;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Set frame format
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fmt frame format
> >> + * @return Error code
> >> + */
> >> +static int sensor_s_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_format *fmt)
> >> +{
> >> + int ret;
> >> + struct sensor_format_struct *sensor_fmt;
> >> + struct sensor_win_size *wsize;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + ov5647_write_array(sd, sensor_oe_disable_regs,
> >> + ARRAY_SIZE(sensor_oe_disable_regs));
> >
> > Should you check the error code here?
> >
> >> +
> >> + ret = sensor_try_fmt_internal(sd, &fmt->format,
> >> + &sensor_fmt, &wsize);
> >
> > Do you set wsize somewhere?
> >
> >> + if (ret)
> >> + return ret;
> >> +
> >> + ov5647_write_array(sd, sensor_fmt->regs, sensor_fmt->regs_size);
> >
> > And here.
> >
> >> +
> >> + ret = 0;
> >
> > ret was already zero.
> >
> >> +
> >> + if (wsize->regs)
> >> + ov5647_write_array(sd, wsize->regs, wsize->regs_size);
> >> +
> >> + if (wsize->set_size)
> >> + wsize->set_size(sd);
> >> +
> >> + info->fmt = sensor_fmt;
> >> + info->width = wsize->width;
> >> + info->height = wsize->height;
> >> +
> >> + ov5647_write_array(sd, sensor_oe_enable_regs,
> >> + ARRAY_SIZE(sensor_oe_enable_regs));
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Set stream parameters
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] parms stream parameters
> >> + * @return Error code
> >> + */
> >> +static int sensor_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >> +{
> >> + struct v4l2_captureparm *cp = &parms->parm.capture;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> + return -EINVAL;
> >> +
> >> + if (info->tpf.numerator == 0)
> >> + return -EINVAL;
> >> +
> >> + info->capture_mode = cp->capturemode;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Get stream parameters
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] parms stream parameters
> >> + * @return Error code
> >> + */
> >> +static int sensor_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
> >> +{
> >> + struct v4l2_captureparm *cp = &parms->parm.capture;
> >> + struct ov5647 *info = to_state(sd);
> >> +
> >> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> >> + return -EINVAL;
> >> +
> >> + memset(cp, 0, sizeof(struct v4l2_captureparm));
> >> + cp->capability = V4L2_CAP_TIMEPERFRAME;
> >> + cp->capturemode = info->capture_mode;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Subdev video operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_video_ops sensor_video_ops = {
> >> + .s_parm = sensor_s_parm,
> >> + .g_parm = sensor_g_parm,
> >
> > Please use the g/s_frame_interval() instead. That's what sub-device drivers
> > generally use for the purpose.
> >
> >> +};
> >> +
> >> +/* ----------------------------------------------------------------------- */
> >> +
> >> +/**
> >> + * @short Subdev operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_ops subdev_ops = {
> >> + .core = &sensor_core_ops,
> >> + .video = &sensor_video_ops,
> >> +};
> >> +
> >> +/* -----------------------------------------------------------------------------
> >> + * V4L2 subdev internal operations
> >> + */
> >> +
> >> +/**
> >> + * @short Detect camera version and model
> >> + * @param[in] sd v4l2 subdev
> >> + * @return Error code
> >> + */
> >> +int ov5647_detect(struct v4l2_subdev *sd)
> >> +{
> >> + unsigned char v;
> >> + int ret;
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
> >> + if (ret < 0)
> >> + return ret;
> >> + ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
> >> + if (ret < 0)
> >> + return ret;
> >> + if (v != 0x56) {
> >> + dev_err(&client->dev, "Wrong model version detected");
> >> + return -ENODEV;
> >> + }
> >> + ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
> >> + if (ret < 0)
> >> + return ret;
> >> + if (v != 0x47) {
> >> + dev_err(&client->dev, "Wrong model version detected");
> >> + return -ENODEV;
> >> + }
> >> +
> >> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Detect if camera is registered
> >> + * @param[in] sd v4l2 subdev
> >> + * @return Error code
> >> + */
> >> +static int ov5647_registered(struct v4l2_subdev *sd)
> >> +{
> >> + struct i2c_client *client = v4l2_get_subdevdata(sd);
> >> +
> >> + dev_info(&client->dev, "OV5647 detected at address 0x%02x\n",
> >> + client->addr);
> >
> > I might omit this. If there's a need to debug this then the information
> > should be printed by the framework instead using debug level messages.
> >
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * @short Open device
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fh v4l2 file handler
> >> + * @return Error code
> >> + */
> >> +static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> >> +{
> >> + struct v4l2_mbus_framefmt *format =
> >> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
> >> + struct v4l2_rect *crop =
> >> + v4l2_subdev_get_try_crop(sd, fh->pad, 0);
> >> +
> >> + crop->left = OV5647_COLUMN_START_DEF;
> >> + crop->top = OV5647_ROW_START_DEF;
> >> + crop->width = OV5647_WINDOW_WIDTH_DEF;
> >> + crop->height = OV5647_WINDOW_HEIGHT_DEF;
> >> +
> >> + format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
> >> +
> >> + format->width = OV5647_WINDOW_WIDTH_DEF;
> >> + format->height = OV5647_WINDOW_HEIGHT_DEF;
> >> + format->field = V4L2_FIELD_NONE;
> >> + format->colorspace = V4L2_COLORSPACE_SRGB;
> >> +
> >> + return sensor_power(sd, true);
> >> +}
> >> +
> >> +/**
> >> + * @short Open device
> >> + * @param[in] sd v4l2 subdev
> >> + * @param[in] fh v4l2 file handler
> >> + * @return Error code
> >> + */
> >> +static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
> >> +{
> >> + return sensor_power(sd, false);
> >> +}
> >> +
> >> +/**
> >> + * @short Subdev internal operations registration
> >> + *
> >> + */
> >> +static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
> >> + .registered = ov5647_registered,
> >> + .open = ov5647_open,
> >> + .close = ov5647_close,
> >> +};
> >> +
> >> +/**
> >> + * @short Initialization routine - Entry point of the driver
> >> + * @param[in] client pointer to the i2c client structure
> >> + * @param[in] id pointer to the i2c device id structure
> >> + * @return 0 on success and a negative number on failure
> >> + */
> >> +static int ov5647_probe(struct i2c_client *client,
> >> + const struct i2c_device_id *id)
> >> +{
> >> + struct device *dev = &client->dev;
> >> + struct ov5647 *sensor;
> >> + int ret = 0;
> >
> > No need to initialise ret here.
> >
> >> + struct v4l2_subdev *sd;
> >> +
> >> + dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
> >> +
> >> + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
> >> + if (sensor == NULL)
> >> + return -ENOMEM;
> >> +
> >> + mutex_init(&sensor->lock);
> >> + sensor->dev = dev;
> >> +
> >> + sd = &sensor->sd;
> >> + v4l2_i2c_subdev_init(sd, client, &subdev_ops);
> >> + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >> +
> >> + sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
> >> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
> >> + ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
> >> + if (ret < 0)
> >> + goto mutex_remove;
> >> +
> >> + ret = ov5647_detect(sd);
> >> + if (ret < 0)
> >> + goto error;
> >> +
> >> + ret = v4l2_async_register_subdev(sd);
> >> + if (ret < 0)
> >> + goto error;
> >> +
> >> + return 0;
> >> +error:
> >> + media_entity_cleanup(&sd->entity);
> >> +mutex_remove:
> >> + mutex_destroy(&sensor->lock);
> >> + return ret;
> >> +}
> >> +
> >> +/**
> >> + * @short Exit routine - Exit point of the driver
> >> + * @param[in] client pointer to the i2c client structure
> >> + * @return 0 on success and a negative number on failure
> >> + */
> >> +static int ov5647_remove(struct i2c_client *client)
> >> +{
> >> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> >> + struct ov5647 *ov5647 = to_state(sd);
> >> +
> >> + v4l2_async_unregister_subdev(&ov5647->sd);
> >> + media_entity_cleanup(&ov5647->sd.entity);
> >> + v4l2_device_unregister_subdev(sd);
> >> + mutex_destroy(&ov5647->lock);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct i2c_device_id ov5647_id[] = {
> >> + { "ov5647", 0 },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, ov5647_id);
> >> +
> >> +#if IS_ENABLED(CONFIG_OF)
> >> +static const struct of_device_id ov5647_of_match[] = {
> >> + { .compatible = "ovti,ov5647" },
> >> + { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, ov5647_of_match);
> >> +#endif
> >> +
> >> +/**
> >> + * @short i2c driver structure
> >> + */
> >> +static struct i2c_driver ov5647_driver = {
> >> + .driver = {
> >> + .of_match_table = of_match_ptr(ov5647_of_match),
> >> + .owner = THIS_MODULE,
> >> + .name = "ov5647",
> >> + },
> >> + .probe = ov5647_probe,
> >> + .remove = ov5647_remove,
> >> + .id_table = ov5647_id,
> >> +};
> >> +
> >> +module_i2c_driver(ov5647_driver);
> >> +
> >> +MODULE_AUTHOR("Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>");
> >> +MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
> >> +MODULE_LICENSE("GPL v2");
> >
--
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@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
* [PATCH v2] PCI: rockchip: Add quirk to disable RC's ASPM L0s
From: Shawn Lin @ 2016-12-12 11:51 UTC (permalink / raw)
To: Bjorn Helgaas, Rob Herring
Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Wenrui Li,
Brian Norris, Jeffy Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
Shawn Lin
Rockchip's RC outputs 100MHz reference clock but there are
two methods for PHY to generate it.
(1)One of them is to use system PLL to generate 100MHz clock and
the PHY will relock it and filter signal noise then outputs the
reference clock.
(2)Another way is to share Soc's 24MHZ crystal oscillator with
PHY and force PHY's DLL to generate 100MHz internally.
When using case(2), the exit from L0s doesn't work fine occasionally
due to the broken design of RC receiver's logical circuit. So even if
we use extended-synch, it still fails for PHY to relock the bits from
FTS sometimes. This will hang the system.
Maybe we could argue that why not use case(1) to avoid it? The reason
is that as we could see the reference clock is derived from system PLL
and the path from it to PHY isn't so clean which means there are some
noise introduced by power-domain and other buses can't be filterd out
by PHY and we could see noise from the frequency spectrum by
oscilloscope. This makes the TX compatibility test a little difficult
to pass the spec. So case(1) and case(2) are both used indeed now. If
using case(2), we should disable RC's L0s support, and that is why we
need this property to indicate this quirk.
Also after checking quirk.c, I noticed there is already a quirk for
disabling L0s unconditionally, quirk_disable_aspm_l0s. But obviously we
shouldn't do that as mentioned above that case(1) could still works fine
with L0s.
Reported-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
Changes in v2:
- drop the quirk prefix
Documentation/devicetree/bindings/pci/rockchip-pcie.txt | 2 ++
drivers/pci/host/pcie-rockchip.c | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
index 71aeda1..1453a73 100644
--- a/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/rockchip-pcie.txt
@@ -43,6 +43,8 @@ Required properties:
- interrupt-map-mask and interrupt-map: standard PCI properties
Optional Property:
+- aspm-no-l0s: RC won't support ASPM L0s. This property is needed if
+ using 24MHz OSC for RC's PHY.
- ep-gpios: contain the entry for pre-reset gpio
- num-lanes: number of lanes to use
- vpcie3v3-supply: The phandle to the 3.3v regulator to use for PCIe.
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index f2dca7b..35988fc 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -140,6 +140,8 @@
#define PCIE_RC_CONFIG_DCR_CSPL_SHIFT 18
#define PCIE_RC_CONFIG_DCR_CSPL_LIMIT 0xff
#define PCIE_RC_CONFIG_DCR_CPLS_SHIFT 26
+#define PCIE_RC_CONFIG_LINK_CAP (PCIE_RC_CONFIG_BASE + 0xcc)
+#define PCIE_RC_CONFIG_LINK_CAP_L0S BIT(10)
#define PCIE_RC_CONFIG_LCS (PCIE_RC_CONFIG_BASE + 0xd0)
#define PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2 (PCIE_RC_CONFIG_BASE + 0x90c)
#define PCIE_RC_CONFIG_THP_CAP (PCIE_RC_CONFIG_BASE + 0x274)
@@ -653,6 +655,13 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
status &= ~PCIE_RC_CONFIG_THP_CAP_NEXT_MASK;
rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_THP_CAP);
+ /* Clear L0s from RC's link cap */
+ if (of_property_read_bool(dev->of_node, "apsm-no-l0s")) {
+ status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_LINK_CAP);
+ status &= ~PCIE_RC_CONFIG_LINK_CAP_L0S;
+ rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_LINK_CAP);
+ }
+
rockchip_pcie_write(rockchip, 0x0, PCIE_RC_BAR_CONF);
rockchip_pcie_write(rockchip,
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Sakari Ailus @ 2016-12-12 11:49 UTC (permalink / raw)
To: Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <cc5229b9-0705-4189-39d5-7c3e0a96c008-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Hi Ramiro,
On Mon, Dec 12, 2016 at 11:39:31AM +0000, Ramiro Oliveira wrote:
> Hi Sakari,
>
> Thank you for the feedback.
>
> On 12/7/2016 10:33 PM, Sakari Ailus wrote:
> > Hi Ramiro,
> >
> > Thank you for the patch.
> >
> > On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
> >> Add device tree documentation.
> >>
> >> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> >> ---
> >> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >> new file mode 100644
> >> index 0000000..4c91b3b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
> >> @@ -0,0 +1,19 @@
> >> +Omnivision OV5647 raw image sensor
> >> +---------------------------------
> >> +
> >> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
> >> +and CCI (I2C compatible) control bus.
> >> +
> >> +Required properties:
> >> +
> >> +- compatible : "ovti,ov5647";
> >> +- reg : I2C slave address of the sensor;
> >> +
> >> +The common video interfaces bindings (see video-interfaces.txt) should be
> >> +used to specify link to the image data receiver. The OV5647 device
> >> +node should contain one 'port' child node with an 'endpoint' subnode.
> >> +
> >> +Following properties are valid for the endpoint node:
> >> +
> >> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> >> + video-interfaces.txt. The sensor supports only two data lanes.
> >
> > Doesn't this sensor require a external clock, a reset GPIO and / or a
> > regulator or a few? Do you need data-lanes, unless you can change the order
> > or the number?
>
> In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
> external clock but it's fixed and not controlled by SW. Should I add a property
> for this?
The sensor datasheet defines a power-up and power-down sequence for the
device. If you don't implement these sequences in the driver on a DT based
system, nothing suggests that they're implemented correctly. Could it be
that the boot loader simply enables the regulators or another device
requires them to be enabled?
I presume at least the reset GPIO should be controlled explicitly in order
to ensure correct function. Although hardware can be surprising: I have one
production system that has no reset GPIO for the sensor albeit the sensor
datasheet says that's part of the power up sequence.
>
> >
> > An example DT snippet wouldn't hurt.
>
> Sure, I can add a example snippet.
>
> >
>
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@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 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:39 UTC (permalink / raw)
To: linux-kernel
Cc: Sebastian Reichel, Rob Herring, Mark Rutland, linux-pm,
devicetree
In-Reply-To: <1481540424-19293-4-git-send-email-peda@axentia.se>
On 2016-12-12 12:00, Peter Rosin wrote:
> If several parallel bq24735 chargers have their ac-detect gpios wired
> together (or if only one of the parallel bq24735 chargers have its
> ac-detect pin wired to a gpio, and the others are assumed to react the
> same), then all driver instances need to check the same gpio. But the
> gpio subsystem does not allow sharing gpios, so handle that locally.
>
> However, only do this for the polling case, sharing is not supported if
> the ac detection is handled with interrupts.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/power/supply/bq24735-charger.c | 101 +++++++++++++++++++++++++++++----
> 1 file changed, 90 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
> index 3765806d5d46..3b21a064a9a7 100644
> --- a/drivers/power/supply/bq24735-charger.c
> +++ b/drivers/power/supply/bq24735-charger.c
> @@ -25,6 +25,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/power_supply.h>
> #include <linux/slab.h>
> @@ -43,12 +44,24 @@
> #define BQ24735_MANUFACTURER_ID 0xfe
> #define BQ24735_DEVICE_ID 0xff
>
> +struct bq24735;
> +
> +struct bq24735_shared {
> + struct list_head list;
> + struct bq24735 *owner;
> + struct gpio_desc *status_gpio;
> +};
> +
> +static struct mutex shared_lock;
Aww crap, that should of course be
static DEFINE_MUTEX(shared_lock);
Will fix in v2, but I'll wait for other feedback first. Why is it
impossible to see these things before hitting send?
Cheers,
peda
^ permalink raw reply
* Re: [PATCH v5 1/2] Add OV5647 device tree documentation
From: Ramiro Oliveira @ 2016-12-12 11:39 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab, linux-kernel, linux-media, robh+dt, devicetree, davem,
gregkh, geert+renesas, akpm, linux, hverkuil, dheitmueller,
slongerbeam, lars, robert.jarzmik, pavel, pali.rohar,
sakari.ailus, mark.rutland, CARLOS.PALMINHA
In-Reply-To: <20161207223319.GZ16630@valkosipuli.retiisi.org.uk>
Hi Sakari,
Thank you for the feedback.
On 12/7/2016 10:33 PM, Sakari Ailus wrote:
> Hi Ramiro,
>
> Thank you for the patch.
>
> On Mon, Dec 05, 2016 at 05:36:33PM +0000, Ramiro Oliveira wrote:
>> Add device tree documentation.
>>
>> Signed-off-by: Ramiro Oliveira <roliveir@synopsys.com>
>> ---
>> .../devicetree/bindings/media/i2c/ov5647.txt | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5647.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5647.txt b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>> new file mode 100644
>> index 0000000..4c91b3b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov5647.txt
>> @@ -0,0 +1,19 @@
>> +Omnivision OV5647 raw image sensor
>> +---------------------------------
>> +
>> +OV5647 is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
>> +and CCI (I2C compatible) control bus.
>> +
>> +Required properties:
>> +
>> +- compatible : "ovti,ov5647";
>> +- reg : I2C slave address of the sensor;
>> +
>> +The common video interfaces bindings (see video-interfaces.txt) should be
>> +used to specify link to the image data receiver. The OV5647 device
>> +node should contain one 'port' child node with an 'endpoint' subnode.
>> +
>> +Following properties are valid for the endpoint node:
>> +
>> +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
>> + video-interfaces.txt. The sensor supports only two data lanes.
>
> Doesn't this sensor require a external clock, a reset GPIO and / or a
> regulator or a few? Do you need data-lanes, unless you can change the order
> or the number?
In the setup I'm using, I'm not aware of any reset GPIO or regulator. I do use a
external clock but it's fixed and not controlled by SW. Should I add a property
for this?
>
> An example DT snippet wouldn't hurt.
Sure, I can add a example snippet.
>
^ permalink raw reply
* Re: [PATCH v5 2/2] Add support for OV5647 sensor
From: Ramiro Oliveira @ 2016-12-12 11:39 UTC (permalink / raw)
To: Sakari Ailus, Ramiro Oliveira
Cc: mchehab-DgEjT+Ai2ygdnm+yROfE0A,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-media-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-0h96xk9xTtrk1uMJSBkQmQ, hverkuil-qWit8jRvyhVmR6Xm/wNWPw,
dheitmueller-eb9eJ82Ua7k9XoPSrs7Ehg,
slongerbeam-Re5JQEeQqe8AvxtiuMwx3w, lars-Qo5EllUWu/uELgA04lAiVw,
robert.jarzmik-GANU6spQydw, pavel-+ZI9xUNit7I,
pali.rohar-Re5JQEeQqe8AvxtiuMwx3w,
sakari.ailus-VuQAYsv1563Yd54FQh9/CA, mark.rutland-5wv7dgnIgG8,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <20161207230138.GA16630-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
Hi Sakari
On 12/7/2016 11:01 PM, Sakari Ailus wrote:
> Hi Ramiro,
>
> On Mon, Dec 05, 2016 at 05:36:34PM +0000, Ramiro Oliveira wrote:
>> Add support for OV5647 sensor.
>>
>> Modes supported:
>> - 640x480 RAW 8
>>
>> Signed-off-by: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> ---
>> MAINTAINERS | 7 +
>> drivers/media/i2c/Kconfig | 12 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/ov5647.c | 866 +++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 886 insertions(+)
>> create mode 100644 drivers/media/i2c/ov5647.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 52cc077..72e828a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8923,6 +8923,13 @@ M: Harald Welte <laforge-TgoAw6mPHtdg9hUCZPvPmw@public.gmane.org>
>> S: Maintained
>> F: drivers/char/pcmcia/cm4040_cs.*
>>
>> +OMNIVISION OV5647 SENSOR DRIVER
>> +M: Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>> +L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +T: git git://linuxtv.org/media_tree.git
>> +S: Maintained
>> +F: drivers/media/i2c/ov5647.c
>> +
>> OMNIVISION OV7670 SENSOR DRIVER
>> M: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
>> L: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index b31fa6f..c1b78e5 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -531,6 +531,18 @@ config VIDEO_OV2659
>> To compile this driver as a module, choose M here: the
>> module will be called ov2659.
>>
>> +config VIDEO_OV5647
>> + tristate "OmniVision OV5647 sensor support"
>> + depends on OF
>
> How does this driver depend on OF, other than matching the compatible
> string?
>
It doesn't, should I proceed diferently?
>> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>> + depends on MEDIA_CAMERA_SUPPORT
>> + ---help---
>> + This is a Video4Linux2 sensor-level driver for the OmniVision
>> + OV5647 camera.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called ov5647.
>> +
>> config VIDEO_OV7640
>> tristate "OmniVision OV7640 sensor support"
>> depends on I2C && VIDEO_V4L2
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 92773b2..0d9014c 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -82,3 +82,4 @@ obj-$(CONFIG_VIDEO_IR_I2C) += ir-kbd-i2c.o
>> obj-$(CONFIG_VIDEO_ML86V7667) += ml86v7667.o
>> obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
>> obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
>> +obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
>> new file mode 100644
>> index 0000000..2aae806
>> --- /dev/null
>> +++ b/drivers/media/i2c/ov5647.c
>> @@ -0,0 +1,866 @@
>> +/*
>> + * A V4L2 driver for OmniVision OV5647 cameras.
>> + *
>> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
>> + * Copyright (C) 2011 Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2006-7 Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
>> + *
>> + * Copyright (C) 2016, Synopsys, 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 version 2.
>> + *
>> + * This program is distributed .as is. WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/i2c.h>
>> +#include <linux/delay.h>
>> +#include <linux/videodev2.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-mediabus.h>
>> +#include <media/v4l2-image-sizes.h>
>> +#include <media/v4l2-of.h>
>> +#include <linux/io.h>
>
> Alphabetical order, please.
>
>> +
>> +#define SENSOR_NAME "ov5647"
>> +
>> +#define OV5647_SW_RESET 0x1003
>> +#define OV5647_REG_CHIPID_H 0x300A
>> +#define OV5647_REG_CHIPID_L 0x300B
>> +
>> +#define REG_TERM 0xfffe
>> +#define VAL_TERM 0xfe
>> +#define REG_DLY 0xffff
>> +
>> +#define OV5647_ROW_START 0x01
>> +#define OV5647_ROW_START_MIN 0
>> +#define OV5647_ROW_START_MAX 2004
>> +#define OV5647_ROW_START_DEF 54
>> +
>> +#define OV5647_COLUMN_START 0x02
>> +#define OV5647_COLUMN_START_MIN 0
>> +#define OV5647_COLUMN_START_MAX 2750
>> +#define OV5647_COLUMN_START_DEF 16
>> +
>> +#define OV5647_WINDOW_HEIGHT 0x03
>> +#define OV5647_WINDOW_HEIGHT_MIN 2
>> +#define OV5647_WINDOW_HEIGHT_MAX 2006
>> +#define OV5647_WINDOW_HEIGHT_DEF 1944
>> +
>> +#define OV5647_WINDOW_WIDTH 0x04
>> +#define OV5647_WINDOW_WIDTH_MIN 2
>> +#define OV5647_WINDOW_WIDTH_MAX 2752
>> +#define OV5647_WINDOW_WIDTH_DEF 2592
>> +
>> +struct regval_list {
>> + u16 addr;
>> + u8 data;
>> +};
>> +
>> +struct cfg_array {
>> + struct regval_list *regs;
>> + int size;
>> +};
>> +
>> +struct sensor_win_size {
>> + int width;
>> + int height;
>> + unsigned int hoffset;
>> + unsigned int voffset;
>> + unsigned int hts;
>> + unsigned int vts;
>> + unsigned int pclk;
>> + unsigned int mipi_bps;
>> + unsigned int fps_fixed;
>> + unsigned int bin_factor;
>> + unsigned int intg_min;
>> + unsigned int intg_max;
>> + void *regs;
>> + int regs_size;
>> + int (*set_size)(struct v4l2_subdev *sd);
>> +};
>> +
>> +
>> +struct ov5647 {
>> + struct device *dev;
>> + struct v4l2_subdev sd;
>> + struct media_pad pad;
>> + struct mutex lock;
>> + struct v4l2_mbus_framefmt format;
>> + struct sensor_format_struct *fmt;
>> + unsigned int width;
>> + unsigned int height;
>> + unsigned int capture_mode;
>> + int hue;
>
> At least capture_mode and hue are unused. Please remove unused fields.
>
>> + struct v4l2_fract tpf;
>> + struct sensor_win_size *current_wins;
>> +};
>> +
>> +static inline struct ov5647 *to_state(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct ov5647, sd);
>> +}
>> +
>> +static struct regval_list sensor_oe_disable_regs[] = {
>> + {0x3000, 0x00},
>> + {0x3001, 0x00},
>> + {0x3002, 0x00},
>> +};
>> +
>> +static struct regval_list sensor_oe_enable_regs[] = {
>> + {0x3000, 0x0f},
>> + {0x3001, 0xff},
>> + {0x3002, 0xe4},
>> +};
>> +
>> +static struct regval_list ov5647_640x480[] = {
>
> Does this list expect a certain external clock frequency? If it does, should
> you check that the actual frequency matches with the expectation?
>
Yes. But like I said in the DT patch the external clock has a fixed frequency. I
can add a check if you thinks it's better.
>> + {0x0100, 0x00},
>> + {0x0103, 0x01},
>> + {0x3034, 0x08},
>> + {0x3035, 0x21},
>> + {0x3036, 0x46},
>> + {0x303c, 0x11},
>> + {0x3106, 0xf5},
>> + {0x3821, 0x07},
>> + {0x3820, 0x41},
>> + {0x3827, 0xec},
>> + {0x370c, 0x0f},
>> + {0x3612, 0x59},
>> + {0x3618, 0x00},
>> + {0x5000, 0x06},
>> + {0x5001, 0x01},
>> + {0x5002, 0x41},
>> + {0x5003, 0x08},
>> + {0x5a00, 0x08},
>> + {0x3000, 0x00},
>> + {0x3001, 0x00},
>> + {0x3002, 0x00},
>> + {0x3016, 0x08},
>> + {0x3017, 0xe0},
>> + {0x3018, 0x44},
>> + {0x301c, 0xf8},
>> + {0x301d, 0xf0},
>> + {0x3a18, 0x00},
>> + {0x3a19, 0xf8},
>> + {0x3c01, 0x80},
>> + {0x3b07, 0x0c},
>> + {0x380c, 0x07},
>> + {0x380d, 0x68},
>> + {0x380e, 0x03},
>> + {0x380f, 0xd8},
>> + {0x3814, 0x31},
>> + {0x3815, 0x31},
>> + {0x3708, 0x64},
>> + {0x3709, 0x52},
>> + {0x3808, 0x02},
>> + {0x3809, 0x80},
>> + {0x380a, 0x01},
>> + {0x380b, 0xE0},
>> + {0x3801, 0x00},
>> + {0x3802, 0x00},
>> + {0x3803, 0x00},
>> + {0x3804, 0x0a},
>> + {0x3805, 0x3f},
>> + {0x3806, 0x07},
>> + {0x3807, 0xa1},
>> + {0x3811, 0x08},
>> + {0x3813, 0x02},
>> + {0x3630, 0x2e},
>> + {0x3632, 0xe2},
>> + {0x3633, 0x23},
>> + {0x3634, 0x44},
>> + {0x3636, 0x06},
>> + {0x3620, 0x64},
>> + {0x3621, 0xe0},
>> + {0x3600, 0x37},
>> + {0x3704, 0xa0},
>> + {0x3703, 0x5a},
>> + {0x3715, 0x78},
>> + {0x3717, 0x01},
>> + {0x3731, 0x02},
>> + {0x370b, 0x60},
>> + {0x3705, 0x1a},
>> + {0x3f05, 0x02},
>> + {0x3f06, 0x10},
>> + {0x3f01, 0x0a},
>> + {0x3a08, 0x01},
>> + {0x3a09, 0x27},
>> + {0x3a0a, 0x00},
>> + {0x3a0b, 0xf6},
>> + {0x3a0d, 0x04},
>> + {0x3a0e, 0x03},
>> + {0x3a0f, 0x58},
>> + {0x3a10, 0x50},
>> + {0x3a1b, 0x58},
>> + {0x3a1e, 0x50},
>> + {0x3a11, 0x60},
>> + {0x3a1f, 0x28},
>> + {0x4001, 0x02},
>> + {0x4004, 0x02},
>> + {0x4000, 0x09},
>> + {0x4837, 0x24},
>> + {0x4050, 0x6e},
>> + {0x4051, 0x8f},
>> + {0x0100, 0x01},
>> +};
>> +
>> +struct sensor_format_struct;
>> +
>> +/**
>> + * @short I2C Write operation
>> + * @param[in] i2c_client I2C client
>> + * @param[in] reg register address
>> + * @param[in] val value to write
>> + * @return Error code
>> + */
>> +static int ov5647_write(struct v4l2_subdev *sd, u16 reg, u8 val)
>> +{
>> + int ret;
>> + unsigned char data[3] = { reg >> 8, reg & 0xff, val};
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = i2c_master_send(client, data, 3);
>> + if (ret != 3) {
>> + dev_dbg(&client->dev, "%s: i2c write error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short I2C Read operation
>> + * @param[in] i2c_client I2C client
>> + * @param[in] reg register address
>> + * @param[out] val value read
>> + * @return Error code
>> + */
>> +static int ov5647_read(struct v4l2_subdev *sd, u16 reg, u8 *val)
>> +{
>> + int ret;
>> + unsigned char data_w[2] = { reg >> 8, reg & 0xff };
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> +
>> + ret = i2c_master_send(client, data_w, 2);
>> +
>> + if (ret < 2) {
>> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> +
>> +
>> + ret = i2c_master_recv(client, val, 1);
>> +
>> + if (ret < 1) {
>> + dev_dbg(&client->dev, "%s: i2c read error, reg: %x\n",
>> + __func__, reg);
>> + return ret < 0 ? ret : -EIO;
>> + }
>> + return 0;
>> +}
>> +
>> +static int ov5647_write_array(struct v4l2_subdev *sd,
>> + struct regval_list *regs, int array_size)
>> +{
>> + int i = 0;
>> + int ret = 0;
>> +
>> + if (!regs)
>> + return -EINVAL;
>> +
>> + while (i < array_size) {
>> + ret = ov5647_write(sd, regs->addr, regs->data);
>> + if (ret < 0)
>> + return ret;
>> + i++;
>> + regs++;
>> + }
>> + return 0;
>> +}
>> +
>> +static void ov5647_set_virtual_channel(struct v4l2_subdev *sd, int channel)
>> +{
>> + u8 channel_id;
>> +
>> + ov5647_read(sd, 0x4814, &channel_id);
>> + channel_id &= ~(3 << 6);
>> + ov5647_write(sd, 0x4814, channel_id | (channel << 6));
>> +}
>> +
>> +void ov5647_stream_on(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ov5647_write(sd, 0x4202, 0x00);
>> + dev_dbg(&client->dev, "Stream on");
>> + ov5647_write(sd, 0x300D, 0x00);
>> +}
>> +
>> +void ov5647_stream_off(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ov5647_write(sd, 0x4202, 0x0f);
>> + dev_dbg(&client->dev, "Stream off");
>> + ov5647_write(sd, 0x300D, 0x01);
>> +}
>> +
>> +/****************************************************************************/
>> +
>> +/**
>> + * @short Set SW standby
>> + * @param[in] sd v4l2 sd
>> + * @param[in] stanby standby mode status (on or off)
>> + * @return Error code
>> + */
>> +static int set_sw_standby(struct v4l2_subdev *sd, bool standby)
>> +{
>> + int ret;
>> + unsigned char rdval;
>> +
>> + ret = ov5647_read(sd, 0x0100, &rdval);
>> + if (ret != 0)
>> + return ret;
>> +
>> + if (standby)
>> + rdval &= 0xfe;
>> + else
>> + rdval |= 0x01;
>> +
>> + ret = ov5647_write(sd, 0x0100, rdval);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Store information about the video data format.
>> + */
>> +static struct sensor_format_struct {
>> + u8 *desc;
>> + u32 mbus_code;
>> + struct regval_list *regs;
>> + int regs_size;
>> + int bpp;
>
> At least desc and bpp are unused.
>
>> +} sensor_formats[] = {
>> + {
>> + .desc = "Raw RGB Bayer",
>> + .mbus_code = MEDIA_BUS_FMT_SBGGR8_1X8,
>> + .regs = ov5647_640x480,
>> + .regs_size = ARRAY_SIZE(ov5647_640x480),
>> + .bpp = 1
>> + },
>> +};
>> +#define N_FMTS ARRAY_SIZE(sensor_formats)
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Initialize sensor
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] val not used
>> + * @return Error code
>> + */
>> +static int __sensor_init(struct v4l2_subdev *sd)
>> +{
>> + int ret;
>> + u8 resetval;
>> + u8 rdval;
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + dev_dbg(&client->dev, "sensor init\n");
>> +
>> + ret = ov5647_read(sd, 0x0100, &rdval);
>> + if (ret != 0)
>> + return ret;
>> +
>> + ov5647_write(sd, 0x4800, 0x25);
>> + ov5647_stream_off(sd);
>> +
>> + ret = ov5647_write_array(sd, ov5647_640x480,
>> + ARRAY_SIZE(ov5647_640x480));
>> + if (ret < 0) {
>> + dev_err(&client->dev, "write sensor_default_regs error\n");
>> + return ret;
>> + }
>> +
>> + ov5647_set_virtual_channel(sd, 0);
>> +
>> + ov5647_read(sd, 0x0100, &resetval);
>> + if (!(resetval & 0x01)) {
>> + dev_err(&client->dev, "Device was in SW standby");
>> + ov5647_write(sd, 0x0100, 0x01);
>> + }
>> +
>> + ov5647_write(sd, 0x4800, 0x04);
>> + ov5647_stream_on(sd);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Control sensor power state
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] on Sensor power
>> + * @return Error code
>> + */
>> +static int sensor_power(struct v4l2_subdev *sd, int on)
>> +{
>> + int ret;
>> + struct ov5647 *ov5647 = to_state(sd);
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = 0;
>> + mutex_lock(&ov5647->lock);
>> +
>> + if (on) {
>> + dev_dbg(&client->dev, "OV5647 power on\n");
>> +
>> + ret = ov5647_write_array(sd, sensor_oe_enable_regs,
>> + ARRAY_SIZE(sensor_oe_enable_regs));
>> +
>> + ret = __sensor_init(sd);
>> +
>> + if (ret < 0)
>> + dev_err(&client->dev,
>> + "Camera not available, check Power\n");
>> + } else {
>> + dev_dbg(&client->dev, "OV5647 power off\n");
>> +
>> + dev_dbg(&client->dev, "disable oe\n");
>> + ret = ov5647_write_array(sd, sensor_oe_disable_regs,
>> + ARRAY_SIZE(sensor_oe_disable_regs));
>> +
>> + if (ret < 0)
>> + dev_dbg(&client->dev, "disable oe failed\n");
>> +
>> + ret = set_sw_standby(sd, true);
>> +
>> + if (ret < 0)
>> + dev_dbg(&client->dev, "soft stby failed\n");
>> +
>> + }
>> +
>> + mutex_unlock(&ov5647->lock);
>> +
>> + return ret;
>> +}
>> +
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> +/**
>> + * @short Get register value
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] reg register struct
>> + * @return Error code
>> + */
>> +static int sensor_get_register(struct v4l2_subdev *sd,
>> + struct v4l2_dbg_register *reg)
>> +{
>> + unsigned char val = 0;
>> + int ret;
>> +
>> + ret = ov5647_read(sd, reg->reg & 0xff, &val);
>> + if (ret != 0)
>> + return ret;
>> +
>> + reg->val = val;
>> + reg->size = 1;
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Set register value
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] reg register struct
>> + * @return Error code
>> + */
>> +static int sensor_set_register(struct v4l2_subdev *sd,
>> + const struct v4l2_dbg_register *reg)
>> +{
>> + return ov5647_write(sd, reg->reg & 0xff, reg->val & 0xff);
>> +}
>> +#endif
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Subdev core operations registration
>> + */
>> +static const struct v4l2_subdev_core_ops sensor_core_ops = {
>> + .s_power = sensor_power,
>
> The s_power() op will be called by the bridge (ISP) driver as well. You
> should expect that it may be enabled more than once before being disabled
> equal number of times.
>
Should I add an IF check to verify if the sensor is powered on before running
the power on/off routine.
>> +#ifdef CONFIG_VIDEO_ADV_DEBUG
>> + .g_register = sensor_get_register,
>> + .s_register = sensor_set_register,
>> +#endif
>> +};
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +
>> +
>> +/**
>> + * @short Enumerate available image formats
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] index index
>> + * @param[in] code MBUS Pixel code
>> + * @return Error code
>> + */
>> +static int sensor_enum_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> + if (code->pad || code->index >= N_FMTS)
>> + return -EINVAL;
>> +
>> + code->code = sensor_formats[code->index].mbus_code;
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Try frame format internal function
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fmt frame format
>> + * @return Error code
>> + */
>> +static int sensor_try_fmt_internal(struct v4l2_subdev *sd,
>> + struct v4l2_mbus_framefmt *fmt, struct sensor_format_struct **ret_fmt,
>> + struct sensor_win_size **ret_wsize)
>> +{
>> + int index;
>> +
>> + for (index = 0; index < N_FMTS; index++)
>> + if (sensor_formats[index].mbus_code == fmt->code)
>> + break;
>> +
>> + if (index >= N_FMTS)
>> + return -EINVAL;
>> +
>> + if (ret_fmt != NULL)
>> + *ret_fmt = sensor_formats + index;
>> +
>> + fmt->field = V4L2_FIELD_NONE;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Set frame format
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fmt frame format
>> + * @return Error code
>> + */
>> +static int sensor_s_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *fmt)
>> +{
>> + int ret;
>> + struct sensor_format_struct *sensor_fmt;
>> + struct sensor_win_size *wsize;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + ov5647_write_array(sd, sensor_oe_disable_regs,
>> + ARRAY_SIZE(sensor_oe_disable_regs));
>
> Should you check the error code here?
>
>> +
>> + ret = sensor_try_fmt_internal(sd, &fmt->format,
>> + &sensor_fmt, &wsize);
>
> Do you set wsize somewhere?
>
>> + if (ret)
>> + return ret;
>> +
>> + ov5647_write_array(sd, sensor_fmt->regs, sensor_fmt->regs_size);
>
> And here.
>
>> +
>> + ret = 0;
>
> ret was already zero.
>
>> +
>> + if (wsize->regs)
>> + ov5647_write_array(sd, wsize->regs, wsize->regs_size);
>> +
>> + if (wsize->set_size)
>> + wsize->set_size(sd);
>> +
>> + info->fmt = sensor_fmt;
>> + info->width = wsize->width;
>> + info->height = wsize->height;
>> +
>> + ov5647_write_array(sd, sensor_oe_enable_regs,
>> + ARRAY_SIZE(sensor_oe_enable_regs));
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Set stream parameters
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] parms stream parameters
>> + * @return Error code
>> + */
>> +static int sensor_s_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> +{
>> + struct v4l2_captureparm *cp = &parms->parm.capture;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + return -EINVAL;
>> +
>> + if (info->tpf.numerator == 0)
>> + return -EINVAL;
>> +
>> + info->capture_mode = cp->capturemode;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Get stream parameters
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] parms stream parameters
>> + * @return Error code
>> + */
>> +static int sensor_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *parms)
>> +{
>> + struct v4l2_captureparm *cp = &parms->parm.capture;
>> + struct ov5647 *info = to_state(sd);
>> +
>> + if (parms->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + return -EINVAL;
>> +
>> + memset(cp, 0, sizeof(struct v4l2_captureparm));
>> + cp->capability = V4L2_CAP_TIMEPERFRAME;
>> + cp->capturemode = info->capture_mode;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Subdev video operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_video_ops sensor_video_ops = {
>> + .s_parm = sensor_s_parm,
>> + .g_parm = sensor_g_parm,
>
> Please use the g/s_frame_interval() instead. That's what sub-device drivers
> generally use for the purpose.
>
>> +};
>> +
>> +/* ----------------------------------------------------------------------- */
>> +
>> +/**
>> + * @short Subdev operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_ops subdev_ops = {
>> + .core = &sensor_core_ops,
>> + .video = &sensor_video_ops,
>> +};
>> +
>> +/* -----------------------------------------------------------------------------
>> + * V4L2 subdev internal operations
>> + */
>> +
>> +/**
>> + * @short Detect camera version and model
>> + * @param[in] sd v4l2 subdev
>> + * @return Error code
>> + */
>> +int ov5647_detect(struct v4l2_subdev *sd)
>> +{
>> + unsigned char v;
>> + int ret;
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x01);
>> + if (ret < 0)
>> + return ret;
>> + ret = ov5647_read(sd, OV5647_REG_CHIPID_H, &v);
>> + if (ret < 0)
>> + return ret;
>> + if (v != 0x56) {
>> + dev_err(&client->dev, "Wrong model version detected");
>> + return -ENODEV;
>> + }
>> + ret = ov5647_read(sd, OV5647_REG_CHIPID_L, &v);
>> + if (ret < 0)
>> + return ret;
>> + if (v != 0x47) {
>> + dev_err(&client->dev, "Wrong model version detected");
>> + return -ENODEV;
>> + }
>> +
>> + ret = ov5647_write(sd, OV5647_SW_RESET, 0x00);
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Detect if camera is registered
>> + * @param[in] sd v4l2 subdev
>> + * @return Error code
>> + */
>> +static int ov5647_registered(struct v4l2_subdev *sd)
>> +{
>> + struct i2c_client *client = v4l2_get_subdevdata(sd);
>> +
>> + dev_info(&client->dev, "OV5647 detected at address 0x%02x\n",
>> + client->addr);
>
> I might omit this. If there's a need to debug this then the information
> should be printed by the framework instead using debug level messages.
>
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * @short Open device
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fh v4l2 file handler
>> + * @return Error code
>> + */
>> +static int ov5647_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_mbus_framefmt *format =
>> + v4l2_subdev_get_try_format(sd, fh->pad, 0);
>> + struct v4l2_rect *crop =
>> + v4l2_subdev_get_try_crop(sd, fh->pad, 0);
>> +
>> + crop->left = OV5647_COLUMN_START_DEF;
>> + crop->top = OV5647_ROW_START_DEF;
>> + crop->width = OV5647_WINDOW_WIDTH_DEF;
>> + crop->height = OV5647_WINDOW_HEIGHT_DEF;
>> +
>> + format->code = MEDIA_BUS_FMT_SBGGR8_1X8;
>> +
>> + format->width = OV5647_WINDOW_WIDTH_DEF;
>> + format->height = OV5647_WINDOW_HEIGHT_DEF;
>> + format->field = V4L2_FIELD_NONE;
>> + format->colorspace = V4L2_COLORSPACE_SRGB;
>> +
>> + return sensor_power(sd, true);
>> +}
>> +
>> +/**
>> + * @short Open device
>> + * @param[in] sd v4l2 subdev
>> + * @param[in] fh v4l2 file handler
>> + * @return Error code
>> + */
>> +static int ov5647_close(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
>> +{
>> + return sensor_power(sd, false);
>> +}
>> +
>> +/**
>> + * @short Subdev internal operations registration
>> + *
>> + */
>> +static const struct v4l2_subdev_internal_ops ov5647_subdev_internal_ops = {
>> + .registered = ov5647_registered,
>> + .open = ov5647_open,
>> + .close = ov5647_close,
>> +};
>> +
>> +/**
>> + * @short Initialization routine - Entry point of the driver
>> + * @param[in] client pointer to the i2c client structure
>> + * @param[in] id pointer to the i2c device id structure
>> + * @return 0 on success and a negative number on failure
>> + */
>> +static int ov5647_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device *dev = &client->dev;
>> + struct ov5647 *sensor;
>> + int ret = 0;
>
> No need to initialise ret here.
>
>> + struct v4l2_subdev *sd;
>> +
>> + dev_info(&client->dev, "Installing OmniVision OV5647 camera driver\n");
>> +
>> + sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
>> + if (sensor == NULL)
>> + return -ENOMEM;
>> +
>> + mutex_init(&sensor->lock);
>> + sensor->dev = dev;
>> +
>> + sd = &sensor->sd;
>> + v4l2_i2c_subdev_init(sd, client, &subdev_ops);
>> + sensor->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> + sensor->pad.flags = MEDIA_PAD_FL_SOURCE;
>> + sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
>> + ret = media_entity_pads_init(&sd->entity, 1, &sensor->pad);
>> + if (ret < 0)
>> + goto mutex_remove;
>> +
>> + ret = ov5647_detect(sd);
>> + if (ret < 0)
>> + goto error;
>> +
>> + ret = v4l2_async_register_subdev(sd);
>> + if (ret < 0)
>> + goto error;
>> +
>> + return 0;
>> +error:
>> + media_entity_cleanup(&sd->entity);
>> +mutex_remove:
>> + mutex_destroy(&sensor->lock);
>> + return ret;
>> +}
>> +
>> +/**
>> + * @short Exit routine - Exit point of the driver
>> + * @param[in] client pointer to the i2c client structure
>> + * @return 0 on success and a negative number on failure
>> + */
>> +static int ov5647_remove(struct i2c_client *client)
>> +{
>> + struct v4l2_subdev *sd = i2c_get_clientdata(client);
>> + struct ov5647 *ov5647 = to_state(sd);
>> +
>> + v4l2_async_unregister_subdev(&ov5647->sd);
>> + media_entity_cleanup(&ov5647->sd.entity);
>> + v4l2_device_unregister_subdev(sd);
>> + mutex_destroy(&ov5647->lock);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct i2c_device_id ov5647_id[] = {
>> + { "ov5647", 0 },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ov5647_id);
>> +
>> +#if IS_ENABLED(CONFIG_OF)
>> +static const struct of_device_id ov5647_of_match[] = {
>> + { .compatible = "ovti,ov5647" },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ov5647_of_match);
>> +#endif
>> +
>> +/**
>> + * @short i2c driver structure
>> + */
>> +static struct i2c_driver ov5647_driver = {
>> + .driver = {
>> + .of_match_table = of_match_ptr(ov5647_of_match),
>> + .owner = THIS_MODULE,
>> + .name = "ov5647",
>> + },
>> + .probe = ov5647_probe,
>> + .remove = ov5647_remove,
>> + .id_table = ov5647_id,
>> +};
>> +
>> +module_i2c_driver(ov5647_driver);
>> +
>> +MODULE_AUTHOR("Ramiro Oliveira <roliveir-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>");
>> +MODULE_DESCRIPTION("A low-level driver for OmniVision ov5647 sensors");
>> +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 3/3] dts: hisi: add dts files for Hi3516CV300 demo board
From: Jiancheng Xue @ 2016-12-12 11:34 UTC (permalink / raw)
To: Pan Wen, robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-I+IVW8TIWO2tmTQ+vhA3Yw, xuwei5-C8/M+/jPZTeaMJb+Lgu22Q,
Arnd Bergmann
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
howell.yang-C8/M+/jPZTeaMJb+Lgu22Q,
jalen.hsu-C8/M+/jPZTeaMJb+Lgu22Q,
lvkuanliang-C8/M+/jPZTeaMJb+Lgu22Q,
suwenping-C8/M+/jPZTeaMJb+Lgu22Q, raojun-C8/M+/jPZTeaMJb+Lgu22Q,
kevin.lixu-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <20161017120705.3726-4-wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On 2016/10/17 20:07, Pan Wen wrote:
> Add dts files for Hi3516CV300 demo board.
>
> Signed-off-by: Pan Wen <wenpan-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
Could you help to review this patch, please?
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/hi3516cv300-demb.dts | 148 ++++++++++++
> arch/arm/boot/dts/hi3516cv300.dtsi | 397 +++++++++++++++++++++++++++++++++
> 3 files changed, 546 insertions(+)
> create mode 100644 arch/arm/boot/dts/hi3516cv300-demb.dts
> create mode 100644 arch/arm/boot/dts/hi3516cv300.dtsi
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index befcd26..1f25530 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -171,6 +171,7 @@ dtb-$(CONFIG_ARCH_HIP01) += \
> dtb-$(CONFIG_ARCH_HIP04) += \
> hip04-d01.dtb
> dtb-$(CONFIG_ARCH_HISI) += \
> + hi3516cv300-demb.dtb \
> hi3519-demb.dtb
> dtb-$(CONFIG_ARCH_HIX5HD2) += \
> hisi-x5hd2-dkb.dtb
> diff --git a/arch/arm/boot/dts/hi3516cv300-demb.dts b/arch/arm/boot/dts/hi3516cv300-demb.dts
> new file mode 100644
> index 0000000..6a75cd6
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300-demb.dts
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +
> +/dts-v1/;
> +#include "hi3516cv300.dtsi"
> +
> +/ {
> + model = "Hisilicon Hi3516CV300 DEMO Board";
> + compatible = "hisilicon,hi3516cv300";
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + serial2 = &uart2;
> + i2c0 = &i2c_bus0;
> + i2c1 = &i2c_bus1;
> + spi0 = &spi_bus0;
> + spi1 = &spi_bus1;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x80000000 0x10000000>;
> + };
> +};
> +
> +&dual_timer0 {
> + status = "okay";
> +};
> +
> +&watchdog {
> + status = "okay";
> +};
> +
> +&pwm {
> + status = "okay";
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&i2c_bus0 {
> + status = "okay";
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pmux>;
> +};
> +
> +&i2c_bus1 {
> + status = "okay";
> + clock-frequency = <100000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pmux>;
> +};
> +
> +&spi_bus0{
> + status = "disabled";
> + num-cs = <1>;
> + cs-gpios = <&gpio_chip0 6 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi0_pmux>;
> +};
> +
> +&spi_bus1{
> + status = "okay";
> + num-cs = <2>;
> + cs-gpios = <&gpio_chip5 3 0>, <&gpio_chip5 4 0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi1_pmux>;
> +};
> +
> +&fmc {
> + spi-nor@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <160000000>;
> + m25p,fast-read;
> + };
> +};
> +
> +&mdio {
> + phy0: phy@1 {
> + reg = <1>;
> + };
> +};
> +
> +&hisi_femac {
> + mac-address = [00 00 00 00 00 00];
> + phy-mode = "rmii";
> + phy-handle = <&phy0>;
> + hisilicon,phy-reset-delays-us = <10000 10000 150000>;
> +};
> +
> +&dmac {
> + status = "okay";
> +};
> +
> +&pmux {
> + i2c0_pmux: i2c0_pmux {
> + pinctrl-single,pins = <
> + 0x2c 0x3
> + 0x30 0x3>;
> + };
> +
> + i2c1_pmux: i2c1_pmux {
> + pinctrl-single,pins = <
> + 0x20 0x1
> + 0x24 0x1>;
> + };
> +
> + spi0_pmux: spi0_pmux {
> + pinctrl-single,pins = <
> + 0x28 0x1
> + 0x2c 0x1
> + 0x30 0x1
> + 0x34 0x1>;
> + };
> +
> + spi1_pmux: spi1_pmux {
> + pinctrl-single,pins = <
> + 0xc4 0x1
> + 0xc8 0x1
> + 0xcc 0x1
> + 0xd0 0x1
> + 0xd4 0x1>;
> + };
> +};
> diff --git a/arch/arm/boot/dts/hi3516cv300.dtsi b/arch/arm/boot/dts/hi3516cv300.dtsi
> new file mode 100644
> index 0000000..1da41ab
> --- /dev/null
> +++ b/arch/arm/boot/dts/hi3516cv300.dtsi
> @@ -0,0 +1,397 @@
> +/*
> + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
> + *
> + * 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.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include "skeleton.dtsi"
> +#include <dt-bindings/clock/hi3516cv300-clock.h>
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,arm926ej-s";
> + reg = <0>;
> + };
> + };
> +
> + vic: interrupt-controller@10040000 {
> + compatible = "arm,pl190-vic";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + reg = <0x10040000 0x1000>;
> + };
> +
> + soc {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "simple-bus";
> + interrupt-parent = <&vic>;
> + ranges;
> +
> + clk_3m: clk_3m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <3000000>;
> + };
> +
> + clk_apb: clk_apb {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + };
> +
> + crg: clock-reset-controller@12010000 {
> + compatible = "hisilicon,hi3516cv300-crg";
> + reg = <0x12010000 0x1000>;
> + #clock-cells = <1>;
> + #reset-cells = <2>;
> + };
> +
> + sysctrl: system-controller@12020000 {
> + compatible = "hisilicon,hi3516cv300-sysctrl", "syscon";
> + reg = <0x12020000 0x1000>;
> + #clock-cells = <1>;
> + };
> +
> + reboot {
> + compatible = "syscon-reboot";
> + regmap = <&sysctrl>;
> + offset = <0x4>;
> + mask = <0xdeadbeef>;
> + };
> +
> + dual_timer0: dual_timer@12000000 {
> + compatible = "arm,sp804", "arm,primecell";
> + reg = <0x12000000 0x1000>;
> + interrupts = <3>;
> + clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> + clock-names = "timer0", "timer1", "apb_pclk";
> + status = "disabled";
> + };
> +
> + dual_timer1: dual_timer@12001000 {
> + compatible = "arm,sp804", "arm,primecell";
> + reg = <0x12001000 0x1000>;
> + interrupts = <4>;
> + clocks = <&clk_3m>, <&clk_3m>, <&clk_apb>;
> + clock-names = "timer0", "timer1", "apb_pclk";
> + status = "disabled";
> + };
> +
> + watchdog: watchdog@12080000 {
> + compatible = "arm,sp805-wdt", "arm,primecell";
> + arm,primecell-periphid = <0x00141805>;
> + reg = <0x12080000 0x1000>;
> + clocks = <&sysctrl HI3516CV300_WDT_CLK>,
> + <&crg HI3516CV300_APB_CLK>;
> + clock-names = "wdog_clk", "apb_pclk";
> + status = "disabled";
> + };
> +
> + pwm: pwm@12130000 {
> + compatible = "hisilicon,hi3516cv300-pwm",
> + "hisilicon,hibvt-pwm";
> + reg = <0x12130000 0x10000>;
> + clocks = <&crg HI3516CV300_PWM_CLK>;
> + resets = <&crg 0x38 0>;
> + #pwm-cells = <2>;
> + status = "disabled";
> + };
> +
> + uart0: uart@12100000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12100000 0x1000>;
> + interrupts = <5>;
> + clocks = <&crg HI3516CV300_UART0_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + uart1: uart@12101000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12101000 0x1000>;
> + interrupts = <30>;
> + clocks = <&crg HI3516CV300_UART1_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + uart2: uart@12102000 {
> + compatible = "arm,pl011", "arm,primecell";
> + reg = <0x12102000 0x1000>;
> + interrupts = <25>;
> + clocks = <&crg HI3516CV300_UART2_CLK>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> + };
> +
> + i2c_bus0: i2c@12110000 {
> + compatible = "hisilicon,hi3516cv300-i2c",
> + "hisilicon,hibvt-i2c";
> + reg = <0x12110000 0x1000>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + status = "disabled";
> + };
> +
> + i2c_bus1: i2c@12112000 {
> + compatible = "hisilicon,hi3516cv300-i2c",
> + "hisilicon,hibvt-i2c";
> + reg = <0x12112000 0x1000>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + status = "disabled";
> + };
> +
> + spi_bus0: spi@12120000 {
> + compatible = "arm,pl022", "arm,primecell";
> + reg = <0x12120000 0x1000>;
> + interrupts = <6>;
> + clocks = <&crg HI3516CV300_SPI0_CLK>;
> + clock-names = "apb_pclk";
> + dmas = <&dmac 12 1>, <&dmac 13 2>;
> + dma-names = "rx", "tx";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + spi_bus1: spi@12121000 {
> + compatible = "arm,pl022", "arm,primecell";
> + reg = <0x12121000 0x1000>, <0x12030000 0x4>;
> + interrupts = <7>;
> + clocks = <&crg HI3516CV300_SPI1_CLK>;
> + clock-names = "apb_pclk";
> + dmas = <&dmac 14 1>, <&dmac 15 2>;
> + dma-names = "rx", "tx";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> +
> + fmc: spi-nor-controller@10000000 {
> + compatible = "hisilicon,fmc-spi-nor";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x10000000 0x1000>, <0x14000000 0x1000000>;
> + reg-names = "control", "memory";
> + clocks = <&crg HI3516CV300_FMC_CLK>;
> + assigned-clocks = <&crg HI3516CV300_FMC_CLK>;
> + assigned-clock-rates = <24000000>;
> + };
> +
> + mdio: mdio@10051100 {
> + compatible = "hisilicon,hisi-femac-mdio";
> + reg = <0x10051100 0x10>;
> + clocks = <&crg HI3516CV300_ETH_CLK>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> +
> + hisi_femac: ethernet@10090000 {
> + compatible = "hisilicon,hi3516cv300-femac",
> + "hisilicon,hisi-femac-v2";
> + reg = <0x10050000 0x1000>,<0x10051300 0x200>;
> + interrupts = <12>;
> + clocks = <&crg HI3516CV300_ETH_CLK>;
> + resets = <&crg 0xec 0>, <&crg 0xec 3>;
> + reset-names = "mac","phy";
> + };
> +
> + dmac: dma-controller@10030000 {
> + compatible = "arm,pl080", "arm,primecell";
> + reg = <0x10030000 0x1000>;
> + interrupts = <14>;
> + clocks = <&crg HI3516CV300_DMAC_CLK>;
> + clock-names = "apb_pclk";
> + lli-bus-interface-ahb1;
> + lli-bus-interface-ahb2;
> + mem-bus-interface-ahb1;
> + mem-bus-interface-ahb2;
> + memcpy-burst-size = <256>;
> + memcpy-bus-width = <32>;
> + #dma-cells = <2>;
> + status = "disabled";
> + };
> +
> + gpio_chip0: gpio@12140000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12140000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 61 2>,
> + <&pmux 4 11 1>,
> + <&pmux 5 10 1>,
> + <&pmux 6 13 2>;
> +
> + status = "disabled";
> + };
> +
> + gpio_chip1: gpio@12141000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12141000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 16 7>,
> + <&pmux 7 0 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip2: gpio@12142000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12142000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 46 1>,
> + <&pmux 1 45 1>,
> + <&pmux 2 44 1>,
> + <&pmux 3 43 1>,
> + <&pmux 4 39 1>,
> + <&pmux 5 38 1>,
> + <&pmux 6 40 1>,
> + <&pmux 7 48 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip3: gpio@12143000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12143000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 37 1>,
> + <&pmux 1 36 1>,
> + <&pmux 2 35 1>,
> + <&pmux 3 34 1>,
> + <&pmux 4 23 2>,
> + <&pmux 6 8 2>;
> + status = "disabled";
> + };
> +
> + gpio_chip4: gpio@12144000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12144000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 27 1>,
> + <&pmux 1 26 1>,
> + <&pmux 2 31 1>,
> + <&pmux 3 30 1>,
> + <&pmux 4 28 2>,
> + <&pmux 6 33 1>,
> + <&pmux 7 32 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip5: gpio@12145000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12145000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 53 1>,
> + <&pmux 1 51 2>,
> + <&pmux 3 50 1>,
> + <&pmux 4 49 1>,
> + <&pmux 5 47 1>,
> + <&pmux 6 40 2>;
> + status = "disabled";
> + };
> +
> + gpio_chip6: gpio@12146000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12146000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 7 1>,
> + <&pmux 1 6 1>,
> + <&pmux 2 4 1>,
> + <&pmux 3 5 1>,
> + <&pmux 4 15 1>,
> + <&pmux 5 1 3>;
> + status = "disabled";
> + };
> +
> + gpio_chip7: gpio@12147000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12147000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 1 55 6>,
> + <&pmux 7 25 1>;
> + status = "disabled";
> + };
> +
> + gpio_chip8: gpio@12148000 {
> + compatible = "arm,pl061", "arm,primecell";
> + reg = <0x12148000 0x1000>;
> + interrupts = <31>;
> + clocks = <&crg HI3516CV300_APB_CLK>;
> + clock-names = "apb_pclk";
> + #gpio-cells = <2>;
> + gpio-ranges = <&pmux 0 63 3>,
> + <&pmux 3 12 1>;
> + status = "disabled";
> + };
> +
> + pmux: pinmux@12040000 {
> + compatible = "pinctrl-single";
> + reg = <0x12040000 0x108>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + #gpio-range-cells = <3>;
> + ranges;
> +
> + pinctrl-single,register-width = <32>;
> + pinctrl-single,function-mask = <7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 54 0
> + &range 55 6 1 &range 61 5 0>;
> +
> + range: gpio-range {
> + #pinctrl-single,gpio-range-cells = <3>;
> + };
> + };
> +
> + pconf: pinconf@12040800 {
> + compatible = "pinconf-single";
> + reg = <0x12040800 0x130>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + pinctrl-single,register-width = <32>;
> + };
> + };
> +};
>
--
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 1/4] net: hix5hd2_gmac: add generic compatible string
From: Dongpo Li @ 2016-12-12 11:16 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland-5wv7dgnIgG8, mturquette-rdvid1DuHRBWk0Htik3J/w,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
zhangfei.gao-QSEj5FYQhm4dnm+yROfE0A,
yisen.zhuang-hv44wF8Li93QT0dZR+AlfA,
salil.mehta-hv44wF8Li93QT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
arnd-r2nGTMty4D4, andrew-g2DYL2Zd6BY,
xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q,
benjamin.chenhao-C8/M+/jPZTeaMJb+Lgu22Q,
caizhiyong-C8/M+/jPZTeaMJb+Lgu22Q, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161209223521.5dnj7l44e663sntl@rob-hp-laptop>
Hi Rob,
On 2016/12/10 6:35, Rob Herring wrote:
> On Mon, Dec 05, 2016 at 09:27:58PM +0800, Dongpo Li wrote:
>> The "hix5hd2" is SoC name, add the generic ethernet driver name.
>> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
>> the SG/TXCSUM/TSO/UFO features.
>>
>> Signed-off-by: Dongpo Li <lidongpo-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
>> ---
>> .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt | 9 +++++++--
>> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 15 +++++++++++----
>> 2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> index 75d398b..75920f0 100644
>> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
>> @@ -1,7 +1,12 @@
>> Hisilicon hix5hd2 gmac controller
>>
>> Required properties:
>> -- compatible: should be "hisilicon,hix5hd2-gmac".
>> +- compatible: should contain one of the following SoC strings:
>> + * "hisilicon,hix5hd2-gemac"
>> + * "hisilicon,hi3798cv200-gemac"
>> + and one of the following version string:
>> + * "hisilicon,hisi-gemac-v1"
>> + * "hisilicon,hisi-gemac-v2"
>
> What combinations are valid? I assume both chips don't have both v1 and
> v2. 2 SoCs and 2 versions so far, I don't think there is much point to
> have the v1 and v2 compatible strings.
>
The v1 and v2 are generic MAC compatible strings, many HiSilicon SoCs may
use the same MAC version. For example,
hix5hd2, hi3716cv200 SoCs use the v1 MAC version,
hi3798cv200, hi3516a SoCs use the v2 MAC version,
and there may be more SoCs added in future.
So I think the generic compatible strings are okay here.
Should I add the hi3716cv200, hi3516a SoCs compatible here?
Do you have any good advice?
>> - reg: specifies base physical address(s) and size of the device registers.
>> The first region is the MAC register base and size.
>> The second region is external interface control register.
>> @@ -20,7 +25,7 @@ Required properties:
>>
>> Example:
>> gmac0: ethernet@f9840000 {
>> - compatible = "hisilicon,hix5hd2-gmac";
>> + compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
>
> You can't just change compatible strings.
>
Okay, maybe I should name all the compatible string with the suffix "-gmac" instead of
"-gemac". This can keep the compatible strings with the same suffix. Is this okay?
Can I just add the generic compatible string without changing the SoCs compatible string?
Like following:
gmac0: ethernet@f9840000 {
- compatible = "hisilicon,hix5hd2-gmac";
+ compatible = "hisilicon,hix5hd2-gmac", "hisilicon,hisi-gmac-v1";
>> reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
>> interrupts = <0 71 4>;
>> #address-cells = <1>;
>
> .
>
Regards,
Dongpo
.
--
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 v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Wolfram Sang @ 2016-12-12 11:10 UTC (permalink / raw)
To: Kachalov Anton
Cc: Brendan Higgins, vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org,
clg-Bxea+6Xhats@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <825541481540788-Mr5SgJCofHtxpj1cXAZ9Bg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
> >> + /* Switch from master mode to slave mode. */
> >> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
> >> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
> >> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
> >> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
> >
> > Can't the hardware work both as master and slave on the same bus?
>
> The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
Thanks! Then the driver should support this. Maybe it is an idea to
first upstream the master support and add the slave support
incrementally?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/2] i2c: aspeed: added driver for Aspeed I2C
From: Kachalov Anton @ 2016-12-12 11:06 UTC (permalink / raw)
To: Wolfram Sang, Brendan Higgins
Cc: vz@mleia.com, clg@kaod.org, robh+dt@kernel.org,
mark.rutland@arm.com, linux-i2c@vger.kernel.org,
devicetree@vger.kernel.org, joel@jms.id.au,
openbmc@lists.ozlabs.org
In-Reply-To: <20161211222622.GK2552@katana>
12.12.2016, 01:26, "Wolfram Sang" <wsa@the-dreams.de>:
> On Tue, Nov 29, 2016 at 05:00:17PM -0800, Brendan Higgins wrote:
>> Added initial master and slave support for Aspeed I2C controller.
>> Supports fourteen busses present in ast24xx and ast25xx BMC SoCs by
>> Aspeed.
>>
>> Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
>
> BTW first the bindings patch please, then the driver.
>
> And one seperate question I just stumbled over:
>
>> + /* Switch from master mode to slave mode. */
>> + func_ctrl_reg_val = aspeed_i2c_read(bus, ASPEED_I2C_FUN_CTRL_REG);
>> + func_ctrl_reg_val &= ~ASPEED_I2CD_MASTER_EN;
>> + func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
>> + aspeed_i2c_write(bus, func_ctrl_reg_val, ASPEED_I2C_FUN_CTRL_REG);
>
> Can't the hardware work both as master and slave on the same bus?
The hardware can work as master and slave on the same bus. This is how IPMB over i2c works on Aspeed.
^ permalink raw reply
* [PATCH 3/3] power: supply: bq24735-charger: allow chargers to share the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
In-Reply-To: <1481540424-19293-1-git-send-email-peda@axentia.se>
If several parallel bq24735 chargers have their ac-detect gpios wired
together (or if only one of the parallel bq24735 chargers have its
ac-detect pin wired to a gpio, and the others are assumed to react the
same), then all driver instances need to check the same gpio. But the
gpio subsystem does not allow sharing gpios, so handle that locally.
However, only do this for the polling case, sharing is not supported if
the ac detection is handled with interrupts.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/power/supply/bq24735-charger.c | 101 +++++++++++++++++++++++++++++----
1 file changed, 90 insertions(+), 11 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 3765806d5d46..3b21a064a9a7 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -25,6 +25,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_gpio.h>
#include <linux/gpio/consumer.h>
#include <linux/power_supply.h>
#include <linux/slab.h>
@@ -43,12 +44,24 @@
#define BQ24735_MANUFACTURER_ID 0xfe
#define BQ24735_DEVICE_ID 0xff
+struct bq24735;
+
+struct bq24735_shared {
+ struct list_head list;
+ struct bq24735 *owner;
+ struct gpio_desc *status_gpio;
+};
+
+static struct mutex shared_lock;
+static LIST_HEAD(shared_list);
+
struct bq24735 {
struct power_supply *charger;
struct power_supply_desc charger_desc;
struct i2c_client *client;
struct bq24735_platform *pdata;
struct mutex lock;
+ struct bq24735_shared *shared;
struct gpio_desc *status_gpio;
struct delayed_work poll;
u32 poll_interval;
@@ -346,6 +359,65 @@ static struct bq24735_platform *bq24735_parse_dt_data(struct i2c_client *client)
return pdata;
}
+static struct gpio_desc *bq24735_get_status_gpio(struct bq24735 *charger)
+{
+ struct device *dev = &charger->client->dev;
+ struct bq24735_shared *shared;
+ int gpio;
+ struct list_head *pos;
+
+ if (!of_property_read_bool(dev->of_node, "ti,ac-detect-gpios"))
+ return NULL;
+
+ gpio = of_get_named_gpio(dev->of_node, "ti,ac-detect-gpios", 0);
+ if (gpio < 0)
+ return ERR_PTR(gpio);
+
+ mutex_lock(&shared_lock);
+ list_for_each(pos, &shared_list) {
+ shared = list_entry(pos, struct bq24735_shared, list);
+ if (gpio != desc_to_gpio(shared->status_gpio))
+ continue;
+
+ get_device(&shared->owner->client->dev);
+ dev_dbg(dev, "sharing gpio with %s\n",
+ shared->owner->pdata->name);
+ charger->shared = shared;
+ mutex_unlock(&shared_lock);
+ return shared->status_gpio;
+ }
+
+ shared = devm_kzalloc(dev, sizeof(*shared), GFP_KERNEL);
+ if (!shared) {
+ mutex_unlock(&shared_lock);
+ return ERR_PTR(-ENOMEM);
+ }
+ shared->owner = charger;
+ shared->status_gpio = gpiod_get(dev, "ti,ac-detect", GPIOD_IN);
+ if (!IS_ERR(shared->status_gpio)) {
+ charger->shared = shared;
+ list_add(&shared->list, &shared_list);
+ }
+ mutex_unlock(&shared_lock);
+
+ return shared->status_gpio;
+}
+
+static void bq24735_put_status_gpio(struct bq24735 *charger)
+{
+ if (!charger->shared)
+ return;
+
+ mutex_lock(&shared_lock);
+ if (charger->shared->owner != charger) {
+ put_device(&charger->shared->owner->client->dev);
+ } else {
+ list_del(&charger->shared->list);
+ gpiod_put(charger->shared->status_gpio);
+ }
+ mutex_unlock(&shared_lock);
+}
+
static int bq24735_charger_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -402,9 +474,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
i2c_set_clientdata(client, charger);
- charger->status_gpio = devm_gpiod_get_optional(&client->dev,
- "ti,ac-detect",
- GPIOD_IN);
+ charger->status_gpio = bq24735_get_status_gpio(charger);
if (IS_ERR(charger->status_gpio)) {
ret = PTR_ERR(charger->status_gpio);
dev_err(&client->dev, "Getting gpio failed: %d\n", ret);
@@ -416,28 +486,30 @@ static int bq24735_charger_probe(struct i2c_client *client,
if (ret < 0) {
dev_err(&client->dev, "Failed to read manufacturer id : %d\n",
ret);
- return ret;
+ goto out;
} else if (ret != 0x0040) {
dev_err(&client->dev,
"manufacturer id mismatch. 0x0040 != 0x%04x\n", ret);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out;
}
ret = bq24735_read_word(client, BQ24735_DEVICE_ID);
if (ret < 0) {
dev_err(&client->dev, "Failed to read device id : %d\n", ret);
- return ret;
+ goto out;
} else if (ret != 0x000B) {
dev_err(&client->dev,
"device id mismatch. 0x000b != 0x%04x\n", ret);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out;
}
}
ret = bq24735_config_charger(charger);
if (ret < 0) {
dev_err(&client->dev, "failed in configuring charger");
- return ret;
+ goto out;
}
/* check for AC adapter presence */
@@ -445,7 +517,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
ret = bq24735_enable_charging(charger);
if (ret < 0) {
dev_err(&client->dev, "Failed to enable charging\n");
- return ret;
+ goto out;
}
}
@@ -455,7 +527,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
ret = PTR_ERR(charger->charger);
dev_err(&client->dev, "Failed to register power supply: %d\n",
ret);
- return ret;
+ goto out;
}
if (client->irq) {
@@ -470,7 +542,7 @@ static int bq24735_charger_probe(struct i2c_client *client,
dev_err(&client->dev,
"Unable to register IRQ %d err %d\n",
client->irq, ret);
- return ret;
+ goto out;
}
} else if (charger->status_gpio) {
ret = of_property_read_u32(client->dev.of_node,
@@ -487,6 +559,11 @@ static int bq24735_charger_probe(struct i2c_client *client,
}
return 0;
+
+out:
+ bq24735_put_status_gpio(charger);
+
+ return ret;
}
static int bq24735_charger_remove(struct i2c_client *client)
@@ -496,6 +573,8 @@ static int bq24735_charger_remove(struct i2c_client *client)
if (charger->poll_interval)
cancel_delayed_work_sync(&charger->poll);
+ bq24735_put_status_gpio(charger);
+
return 0;
}
--
2.1.4
^ permalink raw reply related
* [PATCH 2/3] power: supply: bq24735-charger: optionally poll the ac-detect gpio
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
In-Reply-To: <1481540424-19293-1-git-send-email-peda@axentia.se>
If the ac-detect gpio does not support interrupts, provide a fallback
to poll the gpio at a configurable interval.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
.../bindings/power/supply/ti,bq24735.txt | 2 +
drivers/power/supply/bq24735-charger.c | 50 +++++++++++++++++++---
2 files changed, 47 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
index 3bf55757ceec..59c4dde589cf 100644
--- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
+++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
@@ -25,6 +25,8 @@ Optional properties :
- ti,external-control : Indicates that the charger is configured externally
and that the host should not attempt to enable/disable charging or set the
charge voltage/current.
+ - ti,poll-interval-ms : In case there is no interrupts specified, poll AC
+ presense on the ti,ac-detect-gpios GPIO with this interval.
Example:
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index 1d5c9206e0ed..3765806d5d46 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -50,6 +50,8 @@ struct bq24735 {
struct bq24735_platform *pdata;
struct mutex lock;
struct gpio_desc *status_gpio;
+ struct delayed_work poll;
+ u32 poll_interval;
bool charging;
};
@@ -209,11 +211,8 @@ static int bq24735_charger_is_charging(struct bq24735 *charger)
return !(ret & BQ24735_CHG_OPT_CHARGE_DISABLE);
}
-static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+static void bq24735_update(struct bq24735 *charger)
{
- struct power_supply *psy = devid;
- struct bq24735 *charger = to_bq24735(psy);
-
mutex_lock(&charger->lock);
if (charger->charging && bq24735_charger_is_present(charger))
@@ -223,11 +222,29 @@ static irqreturn_t bq24735_charger_isr(int irq, void *devid)
mutex_unlock(&charger->lock);
- power_supply_changed(psy);
+ power_supply_changed(charger->charger);
+}
+
+static irqreturn_t bq24735_charger_isr(int irq, void *devid)
+{
+ struct power_supply *psy = devid;
+ struct bq24735 *charger = to_bq24735(psy);
+
+ bq24735_update(charger);
return IRQ_HANDLED;
}
+static void bq24735_poll(struct work_struct *work)
+{
+ struct bq24735 *charger = container_of(work, struct bq24735, poll.work);
+
+ bq24735_update(charger);
+
+ schedule_delayed_work(&charger->poll,
+ msecs_to_jiffies(charger->poll_interval));
+}
+
static int bq24735_charger_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -455,11 +472,33 @@ static int bq24735_charger_probe(struct i2c_client *client,
client->irq, ret);
return ret;
}
+ } else if (charger->status_gpio) {
+ ret = of_property_read_u32(client->dev.of_node,
+ "ti,poll-interval-ms",
+ &charger->poll_interval);
+ if (ret)
+ return 0;
+ if (!charger->poll_interval)
+ return 0;
+
+ INIT_DELAYED_WORK(&charger->poll, bq24735_poll);
+ schedule_delayed_work(&charger->poll,
+ msecs_to_jiffies(charger->poll_interval));
}
return 0;
}
+static int bq24735_charger_remove(struct i2c_client *client)
+{
+ struct bq24735 *charger = i2c_get_clientdata(client);
+
+ if (charger->poll_interval)
+ cancel_delayed_work_sync(&charger->poll);
+
+ return 0;
+}
+
static const struct i2c_device_id bq24735_charger_id[] = {
{ "bq24735-charger", 0 },
{}
@@ -478,6 +517,7 @@ static struct i2c_driver bq24735_charger_driver = {
.of_match_table = bq24735_match_ids,
},
.probe = bq24735_charger_probe,
+ .remove = bq24735_charger_remove,
.id_table = bq24735_charger_id,
};
--
2.1.4
^ permalink raw reply related
* [PATCH 1/3] power: supply: bq24735-charger: simplify register update to stop charging
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1481540424-19293-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Providing value bits outside of the mask is pointless.
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
drivers/power/supply/bq24735-charger.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index eb7783b42e0a..1d5c9206e0ed 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -111,8 +111,7 @@ static inline int bq24735_enable_charging(struct bq24735 *charger)
return 0;
return bq24735_update_word(charger->client, BQ24735_CHG_OPT,
- BQ24735_CHG_OPT_CHARGE_DISABLE,
- ~BQ24735_CHG_OPT_CHARGE_DISABLE);
+ BQ24735_CHG_OPT_CHARGE_DISABLE, 0);
}
static inline int bq24735_disable_charging(struct bq24735 *charger)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/3] bq24735-charger: allow gpio polling and sharing
From: Peter Rosin @ 2016-12-12 11:00 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Sebastian Reichel, Rob Herring, Mark Rutland,
linux-pm, devicetree
Hi!
I have a board that features a couple of parallel bq24735 chargers.
However, only one of the chargers has its ACOK pin wired to a gpio,
the other parallel chargers are expected to just behave the same
as the charger that has been singled out like this. Another thing
with the board is that the gpio is not capable of generating an
interrupt.
This series fixes things for me (ok, the first patch was just a
fix for a thing that initially had me confused for a bit, and is
totally unrelated. Ignore if you want to, it's basically just churn).
One thing that I wonder about is if anything should be done to
prevent unloading of the instance that shares its gpio? I thought
about adding a device_link_add() call, but am unsure if that would
be approprite? I'm not unloading drivers at all so it's a total
non-issue for me...
Cheers,
Peter
Peter Rosin (3):
power: supply: bq24735-charger: simplify register update to stop
charging
power: supply: bq24735-charger: optionally poll the ac-detect gpio
power: supply: bq24735-charger: allow chargers to share the ac-detect
gpio
.../bindings/power/supply/ti,bq24735.txt | 2 +
drivers/power/supply/bq24735-charger.c | 154 ++++++++++++++++++---
2 files changed, 138 insertions(+), 18 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH V2 2/2] PM / OPP: Introduce domain-performance-state binding to OPP nodes
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Nishanth Menon, Stephen Boyd
Cc: linaro-kernel, linux-pm, linux-kernel, Vincent Guittot,
Rob Herring, Mark Rutland, Kevin Hilman, Ulf Hansson, Lina Iyer,
devicetree, Nayak Rajendra, Viresh Kumar
In-Reply-To: <cover.1481539827.git.viresh.kumar@linaro.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their nodes directly.
But if the device needs the capability of switching to different domain
performance states, as they may need to support different clock rates,
then the per OPP node can be used to contain that information.
This patch introduces the domain-performance-state (already defined by
Power Domain bindings) to the per OPP node.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/devicetree/bindings/opp/opp.txt | 59 +++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
index 9f5ca4457b5f..43eba7c9fc58 100644
--- a/Documentation/devicetree/bindings/opp/opp.txt
+++ b/Documentation/devicetree/bindings/opp/opp.txt
@@ -154,6 +154,9 @@ properties.
- status: Marks the node enabled/disabled.
+- domain-performance-state: A phandle of a Performance state node as defined by
+ ../power/power_domain.txt binding document.
+
Example 1: Single cluster Dual-core ARM cortex A9, switch DVFS states together.
/ {
@@ -528,3 +531,59 @@ Example 5: opp-supported-hw
};
};
};
+
+Example 7: domain-Performance-state:
+(example: For 1GHz require domain state 1 and for 1.1 & 1.2 GHz require state 2)
+
+/ {
+ foo_domain: power-controller@12340000 {
+ compatible = "foo,power-controller";
+ reg = <0x12340000 0x1000>;
+ #power-domain-cells = <0>;
+ domain-performance-states = <&domain_perf_states>;
+ };
+
+ domain_perf_states: performance_states {
+ compatible = "domain-performance-state";
+ domain_perf_state1: pstate@1 {
+ performance-level = <1>;
+ domain-microvolt = <970000 975000 985000>;
+ };
+ domain_perf_state2: pstate@2 {
+ performance-level = <2>;
+ domain-microvolt = <1000000 1075000 1085000>;
+ };
+ }
+
+ cpus {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cpu@0 {
+ compatible = "arm,cortex-a9";
+ reg = <0>;
+ clocks = <&clk_controller 0>;
+ clock-names = "cpu";
+ operating-points-v2 = <&cpu0_opp_table>;
+ power-domains = <&foo_domain>;
+ };
+ };
+
+ cpu0_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp@1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ domain-performance-state = <&domain_perf_state1>;
+ };
+ opp@1100000000 {
+ opp-hz = /bits/ 64 <1100000000>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+ opp@1200000000 {
+ opp-hz = /bits/ 64 <1200000000>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+ };
+};
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V2 1/2] PM / Domains: Introduce domain-performance-states binding
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Stephen Boyd,
Nishanth Menon, Vincent Guittot, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Lina Iyer, devicetree, Nayak Rajendra,
Viresh Kumar
In-Reply-To: <cover.1481539827.git.viresh.kumar@linaro.org>
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
This patch adds binding to describe the performance states of a power
domain.
If the consumers don't need the capability of switching to different
domain performance states at runtime, then they can simply define their
required domain performance state in their node directly. Otherwise the
consumers can define their requirements with help of other
infrastructure, for example the OPP table.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
.../devicetree/bindings/power/power_domain.txt | 69 ++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
index 723e1ad937da..a456e0dc04e0 100644
--- a/Documentation/devicetree/bindings/power/power_domain.txt
+++ b/Documentation/devicetree/bindings/power/power_domain.txt
@@ -38,6 +38,40 @@ phandle arguments (so called PM domain specifiers) of length specified by the
domain's idle states. In the absence of this property, the domain would be
considered as capable of being powered-on or powered-off.
+- domain-performance-states : A phandle of the performance states node, which
+ defines all the performance states associated with a power
+ domain.
+ The domain-performance-states property reflects the performance states of this
+ PM domain and not the performance states of the devices or sub-domains in the
+ PM domain. Devices and sub-domains have their own performance states, which
+ are dependent on the performance state of the PM domain.
+
+* PM domain performance states node
+
+This describes the performance states of a PM domain.
+
+Required properties:
+- compatible: Allow performance states to express their compatibility. It should
+ be: "domain-performance-state".
+
+- Performance state nodes: This node shall have one or more "Performance State"
+ nodes.
+
+* Performance state node
+
+Required properties:
+- performance-level: A positive integer value representing the performance level
+ associated with a performance state. The integer value '1' represents the
+ lowest performance level and the highest value represents the highest
+ performance level.
+
+Optional properties:
+- domain-microvolt: voltage in micro Volts.
+
+ A single regulator's voltage is specified with an array of size one or three.
+ Single entry is for target voltage and three entries are for <target min max>
+ voltages.
+
Example:
power: power-controller@12340000 {
@@ -118,4 +152,39 @@ The node above defines a typical PM domain consumer device, which is located
inside a PM domain with index 0 of a power controller represented by a node
with the label "power".
+Optional properties:
+- domain-performance-state: A phandle of a Performance state node.
+
+Example:
+
+ parent: power-controller@12340000 {
+ compatible = "foo,power-controller";
+ reg = <0x12340000 0x1000>;
+ #power-domain-cells = <0>;
+ domain-performance-states = <&domain_perf_states>;
+ };
+
+ domain_perf_states: performance_states {
+ compatible = "domain-performance-state";
+ domain_perf_state1: pstate@1 {
+ performance-level = <1>;
+ domain-microvolt = <970000 975000 985000>;
+ };
+ domain_perf_state2: pstate@2 {
+ performance-level = <2>;
+ domain-microvolt = <1000000 1075000 1085000>;
+ };
+ domain_perf_state3: pstate@3 {
+ performance-level = <3>;
+ domain-microvolt = <1100000 1175000 1185000>;
+ };
+ }
+
+ leaky-device@12350000 {
+ compatible = "foo,i-leak-current";
+ reg = <0x12350000 0x1000>;
+ power-domains = <&power 0>;
+ domain-performance-state = <&domain_perf_state2>;
+ };
+
[1]. Documentation/devicetree/bindings/power/domain-idle-state.txt
--
2.7.1.410.g6faf27b
^ permalink raw reply related
* [PATCH V2 0/2] PM / Domains / OPP: Introduce domain-performance-state binding
From: Viresh Kumar @ 2016-12-12 10:56 UTC (permalink / raw)
To: Rafael Wysocki
Cc: linaro-kernel, linux-pm, linux-kernel, Stephen Boyd,
Nishanth Menon, Vincent Guittot, Rob Herring, Mark Rutland,
Kevin Hilman, Ulf Hansson, Lina Iyer, devicetree, Nayak Rajendra,
Viresh Kumar
Hello,
Some platforms have the capability to configure the performance state of
their Power Domains. The performance levels are represented by positive
integer values, a lower value represents lower performance state.
We had some discussions about it in the past on the PM list [1], which is
followed by discussions during the LPC. The outcome of all that was that we
should extend Power Domain framework to support active state power management
as well.
The power-domains until now were only concentrating on the idle state
management of the device and this needs to change in order to reuse the
infrastructure of power domains for active state management.
To get a complete picture of the proposed plan, following is what we
need to do:
- Create DT bindings to get domain performance state information for the
platforms.
- Enhance OPP framework to parse these and call into the PM Qos
framework with a performance state request.
- Enhance PM Qos framework to provide the API to be used by consumers
(or OPP framework) and pass it on to the (Generic) Power Domain
framework.
- Enhance Generic Power Domain framework to accept such requests,
accumulate all belonging to a single power domain and call domain
driver specific callback with the performance state we want for the
domain.
- The domain driver shall then, in a platform specific way, set the
requested performance level.
- Note that these features are applicable to the CPU, GPU and other IIO
or non-IIO devices.
- There can be cases where a device can choose between multiple power
domains based on what performance level we want for the device. In
such cases, we should represent the multiplexer with a separate power
domain. In effect, the device (or OPP table) will correspond to a
single power domain, but the backend driver of that domain shall
implement the multiplexing functionality.
This patchset implements the very first part of this chain and
introduces a new optional property for the consumers of the
power-domains: domain-performance-state. This property can be used
directly by the consumer or its OPP table.
V1->V2:
- The performance states get their own nodes as they can have multiple
values.
- Allow optional property domain-microvolt for the performance states
--
viresh
[1] https://marc.info/?l=linux-pm&m=147747923708075&w=2
Viresh Kumar (2):
PM / Domains: Introduce domain-performance-states binding
PM / OPP: Introduce domain-performance-state binding to OPP nodes
Documentation/devicetree/bindings/opp/opp.txt | 59 ++++++++++++++++++
.../devicetree/bindings/power/power_domain.txt | 69 ++++++++++++++++++++++
2 files changed, 128 insertions(+)
--
2.7.1.410.g6faf27b
^ permalink raw reply
* Re: [PATCH 3/8] rtc: add STM32 RTC driver
From: Amelie DELAUNAY @ 2016-12-12 10:19 UTC (permalink / raw)
To: alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org
Cc: a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Alexandre TORGUE, linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Gabriel FERNANDEZ
In-Reply-To: <bc5c308d-472a-bf05-89b8-6d813fb5321d-qxv4g6HH51o@public.gmane.org>
Hi,
Thanks for the review.
On 12/07/2016 08:08 PM, Alexandre Belloni wrote:
> Hi,
>
> It seems mostly fine.
>
> On 02/12/2016 at 15:09:56 +0100, Amelie Delaunay wrote :
>> This patch adds support for the STM32 RTC.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay-qxv4g6HH51o@public.gmane.org>
>> ---
>> drivers/rtc/Kconfig | 10 +
>> drivers/rtc/Makefile | 1 +
>> drivers/rtc/rtc-stm32.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 788 insertions(+)
>> create mode 100644 drivers/rtc/rtc-stm32.c
>>
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index e859d14..dd8b218 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -1706,6 +1706,16 @@ config RTC_DRV_PIC32
>> This driver can also be built as a module. If so, the module
>> will be called rtc-pic32
>>
>> +config RTC_DRV_STM32
>> + tristate "STM32 On-Chip RTC"
>> + depends on ARCH_STM32
>
> Can you add COMPILE_TEST? Looking at it, nothing seemed to be
> architecture specific and this nicely increases compile test coverage.
>
> It should also probably select REGMAP_MMIO.
>
Ok.
>> diff --git a/drivers/rtc/rtc-stm32.c b/drivers/rtc/rtc-stm32.c
>> new file mode 100644
>> index 0000000..9e710ff
>> --- /dev/null
>> +++ b/drivers/rtc/rtc-stm32.c
>> @@ -0,0 +1,777 @@
>> +/*
>> + * Copyright (C) Amelie Delaunay 2015
>> + * Author: Amelie Delaunay <adelaunay.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> This differs from your SoB. I don't really care but it seems odd.
>
Yes, I've already changed it in my upcoming V2!
>> + * License terms: GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include <linux/bcd.h>
>> +#include <linux/clk.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/ioport.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +#include <linux/rtc.h>
>> +#include <linux/spinlock.h>
>> +
>
> I have the feeling that some of those headers are not necessary maybe
> some cleanup should be done.
>
Ok I'll have a look.
>> +static struct regmap *dbp;
>> +
>> +struct stm32_rtc {
>> + struct rtc_device *rtc_dev;
>> + void __iomem *base;
>> + struct clk *pclk;
>> + struct clk *ck_rtc;
>> + unsigned int clksrc;
>> + spinlock_t lock; /* Protects registers accesses */
>
> That comment makes checkpatch happy but is not super useful :) Anyway...
>
Lack of inspiration :)
>> +static irqreturn_t stm32_rtc_alarm_irq(int irq, void *dev_id)
>> +{
>
> ...can you make that one a threaded IRQ? If that's the case, just take
> the rtc_device mutex here and remove the spinlock. All the other
> function are already protected.
>
Ok I'll study this point.
>> +static int stm32_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>> +{
>> + struct stm32_rtc *rtc = dev_get_drvdata(dev);
>> + struct rtc_time *tm = &alrm->time;
>> + unsigned int alrmar, cr, isr;
>> + unsigned long irqflags;
>> +
>> + spin_lock_irqsave(&rtc->lock, irqflags);
>> +
>> + alrmar = stm32_rtc_readl(rtc, STM32_RTC_ALRMAR);
>> + cr = stm32_rtc_readl(rtc, STM32_RTC_CR);
>> + isr = stm32_rtc_readl(rtc, STM32_RTC_ISR);
>> +
>> + spin_unlock_irqrestore(&rtc->lock, irqflags);
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_DATE_MASK) {
>> + /*
>> + * Date/day don't care in Alarm comparison so alarm triggers
>
> I guess you meant "doesn't matter" (that is also valid for the other
> usages of "don't care".
>
>> + * every day
>> + */
>> + tm->tm_mday = -1;
>> + tm->tm_wday = -1;
>> + } else {
>> + if (alrmar & STM32_RTC_ALRMXR_WDSEL) {
>> + /* Alarm is set to a day of week */
>> + tm->tm_mday = -1;
>> + tm->tm_wday = (alrmar & STM32_RTC_ALRMXR_WDAY) >>
>> + STM32_RTC_ALRMXR_WDAY_SHIFT;
>> + tm->tm_wday %= 7;
>> + } else {
>> + /* Alarm is set to a day of month */
>> + tm->tm_wday = -1;
>> + tm->tm_mday = (alrmar & STM32_RTC_ALRMXR_DATE) >>
>> + STM32_RTC_ALRMXR_DATE_SHIFT;
>> + }
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_HOUR_MASK) {
>> + /* Hours don't care in Alarm comparison */
>> + tm->tm_hour = -1;
>> + } else {
>> + tm->tm_hour = (alrmar & STM32_RTC_ALRMXR_HOUR) >>
>> + STM32_RTC_ALRMXR_HOUR_SHIFT;
>> + if (alrmar & STM32_RTC_ALRMXR_PM)
>> + tm->tm_hour += 12;
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_MIN_MASK) {
>> + /* Minutes don't care in Alarm comparison */
>> + tm->tm_min = -1;
>> + } else {
>> + tm->tm_min = (alrmar & STM32_RTC_ALRMXR_MIN) >>
>> + STM32_RTC_ALRMXR_MIN_SHIFT;
>> + }
>> +
>> + if (alrmar & STM32_RTC_ALRMXR_SEC_MASK) {
>> + /* Seconds don't care in Alarm comparison */
>> + tm->tm_sec = -1;
>> + } else {
>> + tm->tm_sec = (alrmar & STM32_RTC_ALRMXR_SEC) >>
>> + STM32_RTC_ALRMXR_SEC_SHIFT;
>> + }
>> +
> I'm not sure those multiple cases (including STM32_RTC_ALRMXR_WDSEL) are
> useful because the core will always give you valid tm_sec, tm_min,
> tm_hour and tm_mday (it is actually checked up to four times!) so you
> should always end up in the same configuration.
>
> If you think some code other than Linux may set an alarm (e.g. the
> bootloader) then you may keep them in read_alarm but at least you can
> remove them from set_alarm.
>
>
Ok I'll clean this part.
>> +static int stm32_rtc_probe(struct platform_device *pdev)
>> +{
>> + struct stm32_rtc *rtc;
>> + struct resource *res;
>> + int ret;
>> +
>> + rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
>> + if (!rtc)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + rtc->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(rtc->base))
>> + return PTR_ERR(rtc->base);
>> +
>> + dbp = syscon_regmap_lookup_by_phandle(pdev->dev.of_node, "st,syscfg");
>> + if (IS_ERR(dbp)) {
>> + dev_err(&pdev->dev, "no st,syscfg\n");
>> + return PTR_ERR(dbp);
>> + }
>> +
>> + spin_lock_init(&rtc->lock);
>> +
>> + rtc->ck_rtc = devm_clk_get(&pdev->dev, "ck_rtc");
>> + if (IS_ERR(rtc->ck_rtc)) {
>> + dev_err(&pdev->dev, "no ck_rtc clock");
>> + return PTR_ERR(rtc->ck_rtc);
>> + }
>> +
>> + ret = clk_prepare_enable(rtc->ck_rtc);
>> + if (ret)
>> + return ret;
>> +
>> + if (dbp)
>> + regmap_update_bits(dbp, PWR_CR, PWR_CR_DBP, PWR_CR_DBP);
>> +
>> + ret = stm32_rtc_init(pdev, rtc);
>> + if (ret)
>> + goto err;
>> +
>
> Isn't that RTC backuped in some way, do you really need to reinit it
> each time the system reboots?
>
>
Indeed, RTC is backuped. I need to reinit it only if RTC parent clock
has changed.
Best Regards,
Amelie
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* [RFT PATCH] ARM64: dts: meson-gxbb: Add reserved memory zone and usable memory range
From: Neil Armstrong @ 2016-12-12 10:18 UTC (permalink / raw)
To: heinrich.schuchardt, khilman, carlo
Cc: Neil Armstrong, linux-amlogic, linux-arm-kernel, linux-kernel,
devicetree
The Amlogic Meson GXBB secure monitor uses part of the memory space, this
patch adds these reserved zones and redefines the usable memory range for
each boards.
Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 21 +++++++++++++++++++++
.../boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts | 2 +-
.../boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts | 2 +-
.../boot/dts/amlogic/meson-gxl-nexbox-a95x.dts | 2 +-
.../arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 +-
11 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
index 7a078be..ac40b2d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx-p23x-q20x.dtsi
@@ -56,7 +56,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_boot: regulator-vddio_boot {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index fc033c0..e085588 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -55,6 +55,27 @@
#address-cells = <2>;
#size-cells = <2>;
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ secos: secos {
+ reg = <0x0 0x05300000 0x0 0x2000000>;
+ no-map;
+ };
+
+ pstore: pstore {
+ reg = <0x0 0x07300000 0x0 0x100000>;
+ no-map;
+ };
+
+ secmon: secmon {
+ reg = <0x0 0x10000000 0x0 0x200000>;
+ no-map;
+ };
+ };
+
cpus {
#address-cells = <0x2>;
#size-cells = <0x0>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
index 9696820..25b8832 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nexbox-a95x.dts
@@ -62,7 +62,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
leds {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 238fbea..839c66a 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -61,7 +61,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
usb_otg_pwr: regulator-usb-pwrs {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
index 203be28..9a39518 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p20x.dtsi
@@ -55,7 +55,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
usb_pwr: regulator-usb-pwrs {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
index 62fb496..287a4c7 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
index 9a9663a..8bdbbe2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x40000000>;
+ reg = <0x0 0x1000000 0x0 0x3f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
index 2fe167b..2d85295 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
@@ -50,6 +50,6 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
index e99101a..4ec2bbb 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-nexbox-a95x.dts
@@ -60,7 +60,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_card: gpio-regulator {
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
index 9639f01..b8b5b74 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-p212.dts
@@ -59,7 +59,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
index f859d75..1544747 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts
@@ -62,7 +62,7 @@
memory@0 {
device_type = "memory";
- reg = <0x0 0x0 0x0 0x80000000>;
+ reg = <0x0 0x1000000 0x0 0x7f000000>;
};
vddio_boot: regulator-vddio-boot {
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] devicetree: i2c-hid: Add Wacom digitizer + regulator support
From: Benjamin Tissoires @ 2016-12-12 10:01 UTC (permalink / raw)
To: Jiri Kosina
Cc: Rob Herring, Dmitry Torokhov, Doug Anderson, Brian Norris,
Caesar Wang, open list:ARM/Rockchip SoC...,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Mark Rutland
In-Reply-To: <alpine.LNX.2.00.1612120951130.16984@cbobk.fhfr.pm>
On Dec 12 2016 or thereabouts, Jiri Kosina wrote:
> Given the timing (merge window being open) and given then NACK given by
> Rob, I've now unapplied the patches (the for-4.10/i2c-hid branch is now
> obsolete, and has been superseded by for-4.10/i2c-hid-nopower).
>
> However, this is mostly done in order to provide more time for discussion;
> I still disagree with the reasoning behind the NACK.
>
To hopefully make things going forward a little bit, I was wondering
over the week-end if we should not solve this particular issue by adding
an intermediate platform DT node:
instead of having:
---
i2c-hid-dev@2c {
compatible = "hid-over-i2c";
reg = <0x2c>;
hid-descr-addr = <0x0020>;
interrupt-parent = <&gpx3>;
interrupts = <3 2>;
vdd-supply = <sth>;
init-delay-ms = <100>;
};
---
we would have:
---
platform-i2c-hid@01 {
compatible = "very-special-board-that-needs-firmware-quirks-and-delay-of-100ms";
vdd-supply = <sth>;
i2c-hid-dev@2c {
compatible = "hid-over-i2c";
reg = <0x2c>;
hid-descr-addr = <0x0020>;
interrupt-parent = <&gpx3>;
interrupts = <3 2>;
};
};
---
If I am not wrong, the platform device should be initialized before
i2c-hid get called, which allows to setup properly the vdd supply.
On resume/suspend, the tree should be respected and we should be able to
enable/disable power in the same fashion this patch provides.
We could then extend this platform device at will without tinkering in
i2c-hid and we could also handle the GPIOs, reset or whatever is
required in the future through compatibles.
Thoughts? yes? no? bullshit?
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 3/8] rtc: add STM32 RTC driver
From: Amelie DELAUNAY @ 2016-12-12 9:59 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Mathieu Poirier, a.zummo-BfzFCNDTiLLj+vYz1yj4TQ@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
Alexandre TORGUE, linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
rtc-linux-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Gabriel FERNANDEZ
In-Reply-To: <20161207183709.tbkapu34ljlg4skp-m++hUPXGwpdeoWH0uzbU5w@public.gmane.org>
Hi Alexandre,
On 12/07/2016 07:37 PM, Alexandre Belloni wrote:
> On 05/12/2016 at 10:43:14 +0100, Amelie DELAUNAY wrote :
>>>> +
>>>> + device_init_wakeup(&pdev->dev, true);
>>>
>>> What happens if device_init_wakeup() returns an error?
>> It means that RTC won't be able to wake up the board with RTC alarm. I can
>> add a warning for the user in this case ?
>>>
>
> There is exactly one driver ever checking the return value of
> device_init_wakeup(). It basically always succeeds.
>
>
OK so, is it OK for everyone that I bet on the fact that it will always
succeed?
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH v3 1/5] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cédric Le Goater @ 2016-12-12 9:57 UTC (permalink / raw)
To: Joel Stanley, Marek Vasut
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
Brian Norris, Boris Brezillon, Richard Weinberger,
Cyrille Pitchen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland, Brendan Higgins
In-Reply-To: <CACPK8XeTw4uNCjzp7qmbwkojrSAJw_U=7vXD-wTjDL2gb220qQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/12/2016 06:02 AM, Joel Stanley wrote:
> On Sat, Dec 10, 2016 at 3:49 AM, Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:
>
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 4a682ee0f632..5c0efbd9dd89 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -29,6 +29,16 @@ config MTD_SPI_NOR_USE_4K_SECTORS
>> Please note that some tools/drivers/filesystems may not work with
>> 4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>>
>> +config SPI_ASPEED
>> + tristate "Aspeed flash controllers in SPI mode"
>> + depends on ARCH_ASPEED || COMPILE_TEST
>> + depends on HAS_IOMEM && OF
>> + help
>> + This enables support for the Firmware Memory controller (FMC)
>> + in the Aspeed AST2500 SoC when attached to SPI NOR chips,
>> + and support for the SPI flash memory controller (SPI) for
>> + the host firmware. The implementation only supports SPI NOR.
>> +
>> config SPI_ATMEL_QUADSPI
>> tristate "Atmel Quad SPI Controller"
>> depends on ARCH_AT91 || (ARM && COMPILE_TEST)
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 121695e83542..d73d772c689c 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,4 +1,5 @@
>> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>> +obj-$(CONFIG_SPI_ASPEED) += aspeed-smc.o
>
> Can we make the symbol CONFIG_SPI_ASPEED_SMC?
sure. I will change the config option name and leave room for a
potential generic SPI driver.
Thanks,
C.
> Brendan has proposed a generic SPI master driver for the Aspeed SoC
> that would allow the controller to be used for non-flash devices. The
> obvious symbol for that driver is CONFIG_SPI_ASPEED.
>
> Cheers,
>
> Joel
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 3/5] mtd: aspeed: used a label property
From: Cédric Le Goater @ 2016-12-12 9:18 UTC (permalink / raw)
To: Marek Vasut, Joel Stanley
Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, David Woodhouse,
Brian Norris, Boris Brezillon, Richard Weinberger,
Cyrille Pitchen, devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland
In-Reply-To: <d8a6f699-3ee2-07b1-f0f5-478b0d8dd259-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 12/12/2016 03:27 AM, Marek Vasut wrote:
> On 12/12/2016 12:46 AM, Joel Stanley wrote:
>> On Sat, Dec 10, 2016 at 3:49 AM, Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org> wrote:
>>> This can be used to easily identify a chip on a system with multiple
>>> chips.
>>>
>>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>>
>> Our userspace benefits from having this.
>>
>> Acked-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>
> Shouldn't such thing be part of core code then ?
yes. I will merge it in the next version.
we already have the jedec,spi-nor label patch to discuss about
the binding change.
Thanks,
C.
>>> ---
>>> drivers/mtd/spi-nor/aspeed-smc.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>>> index 99302b0d7786..9119c0ca86b6 100644
>>> --- a/drivers/mtd/spi-nor/aspeed-smc.c
>>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>>> @@ -676,6 +676,8 @@ static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>>> nor->prepare = aspeed_smc_prep;
>>> nor->unprepare = aspeed_smc_unprep;
>>>
>>> + mtd->name = of_get_property(child, "label", NULL);
>>> +
>>> ret = aspeed_smc_chip_setup_init(chip, r);
>>> if (ret)
>>> break;
>>> --
>>> 2.7.4
>>>
>
>
--
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 v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver
From: Rick Chang @ 2016-12-12 9:07 UTC (permalink / raw)
To: Ricky Liang
Cc: Hans Verkuil, Laurent Pinchart, Mauro Carvalho Chehab,
Matthias Brugger, Rob Herring, open list, linux-media,
srv_heupstream, moderated list:ARM/Mediatek SoC...,
moderated list:ARM/Mediatek SoC...,
open list:OPEN FIRMWARE AND..., Minghsiu Tsai, Bin Liu
In-Reply-To: <CAAJzSMfNyMiia==mXKo6aBw1VxMBxGE20LB870Zm1u9mCoioyQ@mail.gmail.com>
Hi Ricky,
Thanks for your feedback. We will fix the problem in another patch.
On Mon, 2016-12-12 at 12:34 +0800, Ricky Liang wrote:
> Hi Rick,
>
> On Wed, Nov 30, 2016 at 11:08 AM, Rick Chang <rick.chang@mediatek.com> wrote:
> > Add v4l2 driver for Mediatek JPEG Decoder
> >
> > Signed-off-by: Rick Chang <rick.chang@mediatek.com>
> > Signed-off-by: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
>
> <snip...>
>
> > +static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx,
> > + struct mtk_jpeg_dec_param *param)
> > +{
> > + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > + struct mtk_jpeg_q_data *q_data;
> > +
> > + q_data = &ctx->out_q;
> > + if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
> > + v4l2_dbg(1, debug, &jpeg->v4l2_dev, "Picture size change\n");
> > + return true;
> > + }
> > +
> > + q_data = &ctx->cap_q;
> > + if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> > + MTK_JPEG_FMT_TYPE_CAPTURE)) {
> > + v4l2_dbg(1, debug, &jpeg->v4l2_dev, "format change\n");
> > + return true;
> > + }
> > + return false;
>
> <snip...>
>
> > +static void mtk_jpeg_device_run(void *priv)
> > +{
> > + struct mtk_jpeg_ctx *ctx = priv;
> > + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > + struct vb2_buffer *src_buf, *dst_buf;
> > + enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> > + unsigned long flags;
> > + struct mtk_jpeg_src_buf *jpeg_src_buf;
> > + struct mtk_jpeg_bs bs;
> > + struct mtk_jpeg_fb fb;
> > + int i;
> > +
> > + src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > + dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > + jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(src_buf);
> > +
> > + if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> > + for (i = 0; i < dst_buf->num_planes; i++)
> > + vb2_set_plane_payload(dst_buf, i, 0);
> > + buf_state = VB2_BUF_STATE_DONE;
> > + goto dec_end;
> > + }
> > +
> > + if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
> > + mtk_jpeg_queue_src_chg_event(ctx);
> > + ctx->state = MTK_JPEG_SOURCE_CHANGE;
> > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + return;
> > + }
>
> This only detects source change if multiple OUPUT buffers are queued.
> It does not catch the source change in the following scenario:
>
> - OUPUT buffers for jpeg1 enqueued
> - OUTPUT queue STREAMON
> - userspace creates CAPTURE buffers
> - CAPTURE buffers enqueued
> - CAPTURE queue STREAMON
> - decode
> - OUTPUT queue STREAMOFF
> - userspace recreates OUTPUT buffers for jpeg2
> - OUTPUT buffers for jpeg2 enqueued
> - OUTPUT queue STREAMON
>
> In the above sequence if jpeg2's decoded size is larger than jpeg1 the
> function fails to detect that the existing CAPTURE buffers are not big
> enough to hold the decoded data.
>
> A possible fix is to pass *dst_buf to
> mtk_jpeg_check_resolution_change(), and check in the function that all
> the dst_buf planes are large enough to hold the decoded data.
>
> > +
> > + mtk_jpeg_set_dec_src(ctx, src_buf, &bs);
> > + if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, dst_buf, &fb))
> > + goto dec_end;
> > +
> > + spin_lock_irqsave(&jpeg->hw_lock, flags);
> > + mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> > + mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> > + &jpeg_src_buf->dec_param, &bs, &fb);
> > +
> > + mtk_jpeg_dec_start(jpeg->dec_reg_base);
> > + spin_unlock_irqrestore(&jpeg->hw_lock, flags);
> > + return;
> > +
> > +dec_end:
> > + v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > + v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), buf_state);
> > + v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), buf_state);
> > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > +}
>
> <snip...>
^ 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