* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Kieran Bingham @ 2020-05-14 9:53 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Rob Herring, Jacopo Mondi, Laurent Pinchart,
Niklas Söderlund
In-Reply-To: <20200512174056.GH5526@Mani-XPS-13-9360>
Hi Mani,
On 12/05/2020 18:40, Manivannan Sadhasivam wrote:
> Hi Kieran,
>
> On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:
>> The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
>> CSI-2 output. The device supports multicamera streaming applications,
>> and features the ability to synchronise the attached cameras.
>>
>> CSI-2 output can be configured with 1 to 4 lanes, and a control channel
>> is supported over I2C, which implements an I2C mux to facilitate
>> communications with connected cameras across the reverse control
>> channel.
>>
>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>
>> --
>> v2:
>> - Fix MAINTAINERS entry
>>
>> This posting is released with the following modifications to work
>> without Sakari's VC developments:
>> - max9286_g_mbus_config() re-instated
>> - max9286_get_frame_desc() is not bus/csi aware
>> - max9286_{get,set}_routing() removed
>>
>> v3:
>> - Initialise notifier with v4l2_async_notifier_init
>> - Update for new mbus csi2 format V4L2_MBUS_CSI2_DPHY
>>
>> v4: - Re-introduce required code to function with the VC series.
>>
>> - Implement max9286_get_routing, max9286_set_routing
>> - Remove max9286_g_mbus_config
>>
>> v5: (internal release)
>> - Fix printk formatting for hex value
>> - max9286->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE (add |)
>> - MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER -> MEDIA_ENT_F_VID_IF_BRIDGE
>> - Remove 'device is bound' workaround
>>
>> v6:
>> - v4l2_subdev_krouting instead of v4l2_subdev_routing separated
>> to allow integration without the VC/V4L2-Mux series.
>> - convert sd_to_max9286 to inline function
>> - rename max9286_device to max9286_priv
>> - Cleanup the v4l2_async_notifier
>> - Extend MODULE_AUTHOR
>> - Replace of_graph_get_endpoint_by_regs with fwnode_graph_get_endpoint_by_id
>> - Pass default bus type when parsing fwnode endpoint (Manivannan Sadhasivam)
>> - Use new YAML file reference in MAINTAINERS
>> - Parse new i2c-mux node in max9286_get_i2c_by_id
>> (This could/should be refactored to parse these separately first)
>> - Spelling and calculation fixes in the FSYNC_LOCKED check comments
>> - Identify each enabled i2c-mux channel in a single pass
>> - max9286: Improve mux-state readbility [v2]
>> - Fix frame sync lock durations
>> - Add comment to describe /why/ we must open the mux in s_stream
>> - use -EXDEV as return code for failed link synchronisation.
>> - Fix reference counting of the dt nodeS
>> - Convert to probe_new for I2C
>> - Remove redundant max9286_i2c_mux_state
>> - Provide optional enable-gpio (max9286-pwdn)
>>
>> v7:
>> [Kieran]
>> - Ensure powerdown lines are optional
>> - Add a 4ms power-up delay
>> - Add max9286_check_config_link() to core
>> - Add GPIO chip controller for GPIO0OUT and GPIO1OUT
>> - Fix GPIO registration
>> - max9286: Split out async registration
>> (fixes regulator -EPROBE_DEFERs failures)
>> - Collect all V4L2 registrations
>> - balance v4l2_async refcnting
>> - Rename max9286_v4l2_async_ => max9286_v4l2_notifier_
>>
>> [Jacopo]
>> - Remove redundanct MAXIM_I2C_SPEED macros
>> - Move notifiers operations
>> - Add delay after reverse channel reconfiguration
>> - Move link setup to completion
>> - Fix up max9286_check_config_link() implementation
>> - Remove redundant dual configuration of reverse channel
>>
>> v8:
>>
>> [Kieran]
>> - Update the bound_sources mask on unbind
>> - Convert probe kzalloc usage to devm_ variant
>> - Fix up cleanup path from GPIO PowerDown registration
>> - cleanup GPIO device registration fail path
>> - Convert to use devm_regulator_get()
>> - Fit max9286_parse_dt print on one line
>> - Move multi-device workarounds out of upstream driver
>> - Remove I2C mod-table
>> - Lock format changes
>> - Describe pad index usage
>> - Remove poc_enabled workaround
>> - Rename the max9286_gpio to be more explicit on it's actions.
>> - Move max9286_init_format call
>> - Rework probe sequence and simplify error paths.
>> - Simplify i2c comments
>> - Implement Pixelrate control
>> - Disable overlap window
>>
>> [Jacopo]
>> - Adapt Kconfig to latest upstream changes
>> - Put of node on error
>> - Calculate pixel rate
>> - Simplify overlap window disablement
>>
>> v9:
>>
>> [Kieran]
>> - Kconfig: Depend on OF
>> - Re-sort addition to Makefile
>> ---
>> MAINTAINERS | 10 +
>> drivers/media/i2c/Kconfig | 13 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/max9286.c | 1332 +++++++++++++++++++++++++++++++++++
>> 4 files changed, 1356 insertions(+)
>> create mode 100644 drivers/media/i2c/max9286.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a7bb6e22d5da..99e3bf7760fd 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10274,6 +10274,16 @@ F: Documentation/hwmon/max6697.rst
>> F: drivers/hwmon/max6697.c
>> F: include/linux/platform_data/max6697.h
>>
>> +MAX9286 QUAD GMSL DESERIALIZER DRIVER
>> +M: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> +M: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> +M: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> +M: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>> +L: linux-media@vger.kernel.org
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
>> +F: drivers/media/i2c/max9286.c
>> +
>> MAX9860 MONO AUDIO VOICE CODEC DRIVER
>> M: Peter Rosin <peda@axentia.se>
>> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 3abc80373ec0..2e390f41f6da 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -464,6 +464,19 @@ config VIDEO_VPX3220
>> To compile this driver as a module, choose M here: the
>> module will be called vpx3220.
>>
>> +config VIDEO_MAX9286
>> + tristate "Maxim MAX9286 GMSL deserializer support"
>> + depends on I2C && I2C_MUX
>> + depends on OF
>> + select V4L2_FWNODE
>> + select VIDEO_V4L2_SUBDEV_API
>> + select MEDIA_CONTROLLER
>> + help
>> + This driver supports the Maxim MAX9286 GMSL deserializer.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called max9286.
>> +
>> comment "Video and audio decoders"
>>
>> config VIDEO_SAA717X
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 77bf7d0b691f..f0b001ee4b05 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -117,6 +117,7 @@ obj-$(CONFIG_VIDEO_IMX274) += imx274.o
>> obj-$(CONFIG_VIDEO_IMX290) += imx290.o
>> obj-$(CONFIG_VIDEO_IMX319) += imx319.o
>> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
>> +obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
>> obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
>>
>> obj-$(CONFIG_SDR_MAX2175) += max2175.o
>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
>> new file mode 100644
>> index 000000000000..481d65f2b51d
>> --- /dev/null
>> +++ b/drivers/media/i2c/max9286.c
>> @@ -0,0 +1,1332 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Maxim MAX9286 GMSL Deserializer Driver
>> + *
>> + * Copyright (C) 2017-2019 Jacopo Mondi
>> + * Copyright (C) 2017-2019 Kieran Bingham
>> + * Copyright (C) 2017-2019 Laurent Pinchart
>> + * Copyright (C) 2017-2019 Niklas Söderlund
>> + * Copyright (C) 2016 Renesas Electronics Corporation
>> + * Copyright (C) 2015 Cogent Embedded, Inc.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/fwnode.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/i2c.h>
>> +#include <linux/i2c-mux.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +
>> +#include <media/v4l2-async.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-fwnode.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +/* Register 0x00 */
>> +#define MAX9286_MSTLINKSEL_AUTO (7 << 5)
>> +#define MAX9286_MSTLINKSEL(n) ((n) << 5)
>> +#define MAX9286_EN_VS_GEN BIT(4)
>> +#define MAX9286_LINKEN(n) (1 << (n))
>> +/* Register 0x01 */
>> +#define MAX9286_FSYNCMODE_ECU (3 << 6)
>> +#define MAX9286_FSYNCMODE_EXT (2 << 6)
>> +#define MAX9286_FSYNCMODE_INT_OUT (1 << 6)
>> +#define MAX9286_FSYNCMODE_INT_HIZ (0 << 6)
>> +#define MAX9286_GPIEN BIT(5)
>> +#define MAX9286_ENLMO_RSTFSYNC BIT(2)
>> +#define MAX9286_FSYNCMETH_AUTO (2 << 0)
>> +#define MAX9286_FSYNCMETH_SEMI_AUTO (1 << 0)
>> +#define MAX9286_FSYNCMETH_MANUAL (0 << 0)
>> +#define MAX9286_REG_FSYNC_PERIOD_L 0x06
>> +#define MAX9286_REG_FSYNC_PERIOD_M 0x07
>> +#define MAX9286_REG_FSYNC_PERIOD_H 0x08
>> +/* Register 0x0a */
>> +#define MAX9286_FWDCCEN(n) (1 << ((n) + 4))
>> +#define MAX9286_REVCCEN(n) (1 << (n))
>> +/* Register 0x0c */
>> +#define MAX9286_HVEN BIT(7)
>> +#define MAX9286_EDC_6BIT_HAMMING (2 << 5)
>> +#define MAX9286_EDC_6BIT_CRC (1 << 5)
>> +#define MAX9286_EDC_1BIT_PARITY (0 << 5)
>> +#define MAX9286_DESEL BIT(4)
>> +#define MAX9286_INVVS BIT(3)
>> +#define MAX9286_INVHS BIT(2)
>> +#define MAX9286_HVSRC_D0 (2 << 0)
>> +#define MAX9286_HVSRC_D14 (1 << 0)
>> +#define MAX9286_HVSRC_D18 (0 << 0)
>> +/* Register 0x0f */
>> +#define MAX9286_0X0F_RESERVED BIT(3)
>> +/* Register 0x12 */
>> +#define MAX9286_CSILANECNT(n) (((n) - 1) << 6)
>> +#define MAX9286_CSIDBL BIT(5)
>> +#define MAX9286_DBL BIT(4)
>> +#define MAX9286_DATATYPE_USER_8BIT (11 << 0)
>> +#define MAX9286_DATATYPE_USER_YUV_12BIT (10 << 0)
>> +#define MAX9286_DATATYPE_USER_24BIT (9 << 0)
>> +#define MAX9286_DATATYPE_RAW14 (8 << 0)
>> +#define MAX9286_DATATYPE_RAW11 (7 << 0)
>> +#define MAX9286_DATATYPE_RAW10 (6 << 0)
>> +#define MAX9286_DATATYPE_RAW8 (5 << 0)
>> +#define MAX9286_DATATYPE_YUV422_10BIT (4 << 0)
>> +#define MAX9286_DATATYPE_YUV422_8BIT (3 << 0)
>> +#define MAX9286_DATATYPE_RGB555 (2 << 0)
>> +#define MAX9286_DATATYPE_RGB565 (1 << 0)
>> +#define MAX9286_DATATYPE_RGB888 (0 << 0)
>> +/* Register 0x15 */
>> +#define MAX9286_VC(n) ((n) << 5)
>> +#define MAX9286_VCTYPE BIT(4)
>> +#define MAX9286_CSIOUTEN BIT(3)
>> +#define MAX9286_0X15_RESV (3 << 0)
>> +/* Register 0x1b */
>> +#define MAX9286_SWITCHIN(n) (1 << ((n) + 4))
>> +#define MAX9286_ENEQ(n) (1 << (n))
>> +/* Register 0x27 */
>> +#define MAX9286_LOCKED BIT(7)
>> +/* Register 0x31 */
>> +#define MAX9286_FSYNC_LOCKED BIT(6)
>> +/* Register 0x34 */
>> +#define MAX9286_I2CLOCACK BIT(7)
>> +#define MAX9286_I2CSLVSH_1046NS_469NS (3 << 5)
>> +#define MAX9286_I2CSLVSH_938NS_352NS (2 << 5)
>> +#define MAX9286_I2CSLVSH_469NS_234NS (1 << 5)
>> +#define MAX9286_I2CSLVSH_352NS_117NS (0 << 5)
>> +#define MAX9286_I2CMSTBT_837KBPS (7 << 2)
>> +#define MAX9286_I2CMSTBT_533KBPS (6 << 2)
>> +#define MAX9286_I2CMSTBT_339KBPS (5 << 2)
>> +#define MAX9286_I2CMSTBT_173KBPS (4 << 2)
>> +#define MAX9286_I2CMSTBT_105KBPS (3 << 2)
>> +#define MAX9286_I2CMSTBT_84KBPS (2 << 2)
>> +#define MAX9286_I2CMSTBT_28KBPS (1 << 2)
>> +#define MAX9286_I2CMSTBT_8KBPS (0 << 2)
>> +#define MAX9286_I2CSLVTO_NONE (3 << 0)
>> +#define MAX9286_I2CSLVTO_1024US (2 << 0)
>> +#define MAX9286_I2CSLVTO_256US (1 << 0)
>> +#define MAX9286_I2CSLVTO_64US (0 << 0)
>> +/* Register 0x3b */
>> +#define MAX9286_REV_TRF(n) ((n) << 4)
>> +#define MAX9286_REV_AMP(n) ((((n) - 30) / 10) << 1) /* in mV */
>> +#define MAX9286_REV_AMP_X BIT(0)
>> +/* Register 0x3f */
>> +#define MAX9286_EN_REV_CFG BIT(6)
>> +#define MAX9286_REV_FLEN(n) ((n) - 20)
>> +/* Register 0x49 */
>> +#define MAX9286_VIDEO_DETECT_MASK 0x0f
>> +/* Register 0x69 */
>> +#define MAX9286_LFLTBMONMASKED BIT(7)
>> +#define MAX9286_LOCKMONMASKED BIT(6)
>> +#define MAX9286_AUTOCOMBACKEN BIT(5)
>> +#define MAX9286_AUTOMASKEN BIT(4)
>> +#define MAX9286_MASKLINK(n) ((n) << 0)
>> +
>> +/*
>> + * The sink and source pads are created to match the OF graph port numbers so
>> + * that their indexes can be used interchangeably.
>> + */
>> +#define MAX9286_NUM_GMSL 4
>> +#define MAX9286_N_SINKS 4
>> +#define MAX9286_N_PADS 5
>> +#define MAX9286_SRC_PAD 4
>> +
>> +struct max9286_source {
>> + struct v4l2_async_subdev asd;
>> + struct v4l2_subdev *sd;
>> + struct fwnode_handle *fwnode;
>> +};
>> +
>> +#define asd_to_max9286_source(_asd) \
>> + container_of(_asd, struct max9286_source, asd)
>> +
>> +struct max9286_priv {
>> + struct i2c_client *client;
>> + struct gpio_desc *gpiod_pwdn;
>> + struct v4l2_subdev sd;
>> + struct media_pad pads[MAX9286_N_PADS];
>> + struct regulator *regulator;
>> +
>> + struct gpio_chip gpio;
>> + u8 gpio_state;
>> +
>> + struct i2c_mux_core *mux;
>> + unsigned int mux_channel;
>> + bool mux_open;
>> +
>> + struct v4l2_ctrl_handler ctrls;
>> + struct v4l2_ctrl *pixelrate;
>> +
>> + struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
>> +
>> + /* Protects controls and fmt structures */
>> + struct mutex mutex;
>> +
>> + unsigned int nsources;
>> + unsigned int source_mask;
>> + unsigned int route_mask;
>> + unsigned int bound_sources;
>> + unsigned int csi2_data_lanes;
>> + struct max9286_source sources[MAX9286_NUM_GMSL];
>> + struct v4l2_async_notifier notifier;
>> +};
>> +
>> +static struct max9286_source *next_source(struct max9286_priv *priv,
>> + struct max9286_source *source)
>> +{
>> + if (!source)
>> + source = &priv->sources[0];
>> + else
>> + source++;
>> +
>> + for (; source < &priv->sources[MAX9286_NUM_GMSL]; source++) {
>> + if (source->fwnode)
>> + return source;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +#define for_each_source(priv, source) \
>> + for ((source) = NULL; ((source) = next_source((priv), (source))); )
>> +
>> +#define to_index(priv, source) ((source) - &(priv)->sources[0])
>> +
>> +static inline struct max9286_priv *sd_to_max9286(struct v4l2_subdev *sd)
>> +{
>> + return container_of(sd, struct max9286_priv, sd);
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * I2C IO
>> + */
>> +
>> +static int max9286_read(struct max9286_priv *priv, u8 reg)
>> +{
>> + int ret;
>> +
>> + ret = i2c_smbus_read_byte_data(priv->client, reg);
>> + if (ret < 0)
>> + dev_err(&priv->client->dev,
>> + "%s: register 0x%02x read failed (%d)\n",
>> + __func__, reg, ret);
>> +
>> + return ret;
>> +}
>> +
>> +static int max9286_write(struct max9286_priv *priv, u8 reg, u8 val)
>> +{
>> + int ret;
>> +
>> + ret = i2c_smbus_write_byte_data(priv->client, reg, val);
>> + if (ret < 0)
>> + dev_err(&priv->client->dev,
>> + "%s: register 0x%02x write failed (%d)\n",
>> + __func__, reg, ret);
>> +
>> + return ret;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * I2C Multiplexer
>> + */
>> +
>> +static void max9286_i2c_mux_configure(struct max9286_priv *priv, u8 conf)
>> +{
>> + max9286_write(priv, 0x0a, conf);
>> +
>> + /*
>> + * We must sleep after any change to the forward or reverse channel
>> + * configuration.
>> + */
>> + usleep_range(3000, 5000);
>> +}
>> +
>> +static void max9286_i2c_mux_open(struct max9286_priv *priv)
>> +{
>> + /* Open all channels on the MAX9286 */
>> + max9286_i2c_mux_configure(priv, 0xff);
>> +
>> + priv->mux_open = true;
>> +}
>> +
>> +static void max9286_i2c_mux_close(struct max9286_priv *priv)
>> +{
>> + /*
>> + * Ensure that both the forward and reverse channel are disabled on the
>> + * mux, and that the channel ID is invalidated to ensure we reconfigure
>> + * on the next max9286_i2c_mux_select() call.
>> + */
>> + max9286_i2c_mux_configure(priv, 0x00);
>> +
>> + priv->mux_open = false;
>> + priv->mux_channel = -1;
>> +}
>> +
>> +static int max9286_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
>> +{
>> + struct max9286_priv *priv = i2c_mux_priv(muxc);
>> +
>> + /* Channel select is disabled when configured in the opened state. */
>> + if (priv->mux_open)
>> + return 0;
>> +
>> + if (priv->mux_channel == chan)
>> + return 0;
>> +
>> + priv->mux_channel = chan;
>> +
>> + max9286_i2c_mux_configure(priv,
>> + MAX9286_FWDCCEN(chan) |
>> + MAX9286_REVCCEN(chan));
>> +
>> + return 0;
>> +}
>> +
>> +static int max9286_i2c_mux_init(struct max9286_priv *priv)
>> +{
>> + struct max9286_source *source;
>> + int ret;
>> +
>> + if (!i2c_check_functionality(priv->client->adapter,
>> + I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
>> + return -ENODEV;
>> +
>> + priv->mux = i2c_mux_alloc(priv->client->adapter, &priv->client->dev,
>> + priv->nsources, 0, I2C_MUX_LOCKED,
>> + max9286_i2c_mux_select, NULL);
>> + if (!priv->mux)
>> + return -ENOMEM;
>> +
>> + priv->mux->priv = priv;
>> +
>> + for_each_source(priv, source) {
>> + unsigned int index = to_index(priv, source);
>> +
>> + ret = i2c_mux_add_adapter(priv->mux, 0, index, 0);
>> + if (ret < 0)
>> + goto error;
>> + }
>> +
>> + return 0;
>> +
>> +error:
>> + i2c_mux_del_adapters(priv->mux);
>> + return ret;
>> +}
>> +
>> +static void max9286_configure_i2c(struct max9286_priv *priv, bool localack)
>> +{
>> + u8 config = MAX9286_I2CSLVSH_469NS_234NS | MAX9286_I2CSLVTO_1024US |
>> + MAX9286_I2CMSTBT_105KBPS;
>> +
>> + if (localack)
>> + config |= MAX9286_I2CLOCACK;
>> +
>> + max9286_write(priv, 0x34, config);
>> + usleep_range(3000, 5000);
>> +}
>> +
>> +/*
>> + * max9286_check_video_links() - Make sure video links are detected and locked
>> + *
>> + * Performs safety checks on video link status. Make sure they are detected
>> + * and all enabled links are locked.
>> + *
>> + * Returns 0 for success, -EIO for errors.
>> + */
>> +static int max9286_check_video_links(struct max9286_priv *priv)
>> +{
>> + unsigned int i;
>> + int ret;
>> +
>> + /*
>> + * Make sure valid video links are detected.
>> + * The delay is not characterized in de-serializer manual, wait up
>> + * to 5 ms.
>> + */
>> + for (i = 0; i < 10; i++) {
>> + ret = max9286_read(priv, 0x49);
>> + if (ret < 0)
>> + return -EIO;
>> +
>> + if ((ret & MAX9286_VIDEO_DETECT_MASK) == priv->source_mask)
>> + break;
>> +
>> + usleep_range(350, 500);
>> + }
>> +
>> + if (i == 10) {
>> + dev_err(&priv->client->dev,
>> + "Unable to detect video links: 0x%02x\n", ret);
>> + return -EIO;
>> + }
>> +
>> + /* Make sure all enabled links are locked (4ms max). */
>> + for (i = 0; i < 10; i++) {
>> + ret = max9286_read(priv, 0x27);
>> + if (ret < 0)
>> + return -EIO;
>> +
>> + if (ret & MAX9286_LOCKED)
>> + break;
>> +
>> + usleep_range(350, 450);
>> + }
>> +
>> + if (i == 10) {
>> + dev_err(&priv->client->dev, "Not all enabled links locked\n");
>> + return -EIO;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * max9286_check_config_link() - Detect and wait for configuration links
>> + *
>> + * Determine if the configuration channel is up and settled for a link.
>> + *
>> + * Returns 0 for success, -EIO for errors.
>> + */
>> +static int max9286_check_config_link(struct max9286_priv *priv,
>> + unsigned int source_mask)
>> +{
>> + unsigned int conflink_mask = (source_mask & 0x0f) << 4;
>> + unsigned int i;
>> + int ret;
>> +
>> + /*
>> + * Make sure requested configuration links are detected.
>> + * The delay is not characterized in the chip manual: wait up
>> + * to 5 milliseconds.
>> + */
>> + for (i = 0; i < 10; i++) {
>> + ret = max9286_read(priv, 0x49) & 0xf0;
>> + if (ret < 0)
>> + return -EIO;
>> +
>> + if (ret == conflink_mask)
>> + break;
>> +
>> + usleep_range(350, 500);
>> + }
>> +
>> + if (ret != conflink_mask) {
>> + dev_err(&priv->client->dev,
>> + "Unable to detect configuration links: 0x%02x expected 0x%02x\n",
>> + ret, conflink_mask);
>> + return -EIO;
>> + }
>> +
>> + dev_info(&priv->client->dev,
>> + "Successfully detected configuration links after %u loops: 0x%02x\n",
>> + i, conflink_mask);
>> +
>> + return 0;
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * V4L2 Subdev
>> + */
>> +
>> +static int max9286_set_pixelrate(struct max9286_priv *priv)
>> +{
>> + struct max9286_source *source = NULL;
>> + u64 pixelrate = 0;
>> +
>> + for_each_source(priv, source) {
>> + struct v4l2_ctrl *ctrl;
>> + u64 source_rate = 0;
>> +
>> + /* Pixel rate is mandatory to be reported by sources. */
>> + ctrl = v4l2_ctrl_find(source->sd->ctrl_handler,
>> + V4L2_CID_PIXEL_RATE);
>> + if (!ctrl) {
>> + pixelrate = 0;
>> + break;
>> + }
>> +
>> + /* All source must report the same pixel rate. */
>> + source_rate = v4l2_ctrl_g_ctrl_int64(ctrl);
>> + if (!pixelrate) {
>> + pixelrate = source_rate;
>> + } else if (pixelrate != source_rate) {
>> + dev_err(&priv->client->dev,
>> + "Unable to calculate pixel rate\n");
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + if (!pixelrate) {
>> + dev_err(&priv->client->dev,
>> + "No pixel rate control available in sources\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * The CSI-2 transmitter pixel rate is the single source rate multiplied
>> + * by the number of available sources.
>> + */
>> + return v4l2_ctrl_s_ctrl_int64(priv->pixelrate,
>> + pixelrate * priv->nsources);
>> +}
>> +
>> +static int max9286_notify_bound(struct v4l2_async_notifier *notifier,
>> + struct v4l2_subdev *subdev,
>> + struct v4l2_async_subdev *asd)
>> +{
>> + struct max9286_priv *priv = sd_to_max9286(notifier->sd);
>> + struct max9286_source *source = asd_to_max9286_source(asd);
>> + unsigned int index = to_index(priv, source);
>> + unsigned int src_pad;
>> + int ret;
>> +
>> + ret = media_entity_get_fwnode_pad(&subdev->entity,
>> + source->fwnode,
>> + MEDIA_PAD_FL_SOURCE);
>> + if (ret < 0) {
>> + dev_err(&priv->client->dev,
>> + "Failed to find pad for %s\n", subdev->name);
>> + return ret;
>> + }
>> +
>> + priv->bound_sources |= BIT(index);
>> + source->sd = subdev;
>> + src_pad = ret;
>> +
>> + ret = media_create_pad_link(&source->sd->entity, src_pad,
>> + &priv->sd.entity, index,
>> + MEDIA_LNK_FL_ENABLED |
>> + MEDIA_LNK_FL_IMMUTABLE);
>> + if (ret) {
>> + dev_err(&priv->client->dev,
>> + "Unable to link %s:%u -> %s:%u\n",
>> + source->sd->name, src_pad, priv->sd.name, index);
>> + return ret;
>> + }
>> +
>> + dev_dbg(&priv->client->dev, "Bound %s pad: %u on index %u\n",
>> + subdev->name, src_pad, index);
>> +
>> + /*
>> + * We can only register v4l2_async_notifiers, which do not provide a
>> + * means to register a complete callback. bound_sources allows us to
>> + * identify when all remote serializers have completed their probe.
>> + */
>> + if (priv->bound_sources != priv->source_mask)
>> + return 0;
>> +
>> + /*
>> + * All enabled sources have probed and enabled their reverse control
>> + * channels:
>> + *
>> + * - Verify all configuration links are properly detected
>> + * - Disable auto-ack as communication on the control channel are now
>> + * stable.
>> + */
>> + max9286_check_config_link(priv, priv->source_mask);
>> +
>> + /*
>> + * Re-configure I2C with local acknowledge disabled after cameras have
>> + * probed.
>> + */
>> + max9286_configure_i2c(priv, false);
>> +
>> + return max9286_set_pixelrate(priv);
>> +}
>> +
>> +static void max9286_notify_unbind(struct v4l2_async_notifier *notifier,
>> + struct v4l2_subdev *subdev,
>> + struct v4l2_async_subdev *asd)
>> +{
>> + struct max9286_priv *priv = sd_to_max9286(notifier->sd);
>> + struct max9286_source *source = asd_to_max9286_source(asd);
>> + unsigned int index = to_index(priv, source);
>> +
>> + source->sd = NULL;
>> + priv->bound_sources &= ~BIT(index);
>> +}
>> +
>> +static const struct v4l2_async_notifier_operations max9286_notify_ops = {
>> + .bound = max9286_notify_bound,
>> + .unbind = max9286_notify_unbind,
>> +};
>> +
>> +static int max9286_v4l2_notifier_register(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct max9286_source *source = NULL;
>> + int ret;
>> +
>> + if (!priv->nsources)
>> + return 0;
>> +
>> + v4l2_async_notifier_init(&priv->notifier);
>> +
>> + for_each_source(priv, source) {
>> + unsigned int i = to_index(priv, source);
>> +
>> + source->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
>> + source->asd.match.fwnode = source->fwnode;
>> +
>> + ret = v4l2_async_notifier_add_subdev(&priv->notifier,
>> + &source->asd);
>> + if (ret) {
>> + dev_err(dev, "Failed to add subdev for source %d", i);
>> + v4l2_async_notifier_cleanup(&priv->notifier);
>> + return ret;
>> + }
>> +
>> + /*
>> + * Balance the reference counting handled through
>> + * v4l2_async_notifier_cleanup()
>> + */
>> + fwnode_handle_get(source->fwnode);
>> + }
>> +
>> + priv->notifier.ops = &max9286_notify_ops;
>> +
>> + ret = v4l2_async_subdev_notifier_register(&priv->sd, &priv->notifier);
>> + if (ret) {
>> + dev_err(dev, "Failed to register subdev_notifier");
>> + v4l2_async_notifier_cleanup(&priv->notifier);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void max9286_v4l2_notifier_unregister(struct max9286_priv *priv)
>> +{
>> + if (!priv->nsources)
>> + return;
>> +
>> + v4l2_async_notifier_unregister(&priv->notifier);
>> + v4l2_async_notifier_cleanup(&priv->notifier);
>> +}
>> +
>> +static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
>> +{
>> + struct max9286_priv *priv = sd_to_max9286(sd);
>> + struct max9286_source *source;
>> + unsigned int i;
>> + bool sync = false;
>> + int ret;
>> +
>> + if (enable) {
>> + /*
>> + * The frame sync between cameras is transmitted across the
>> + * reverse channel as GPIO. We must open all channels while
>> + * streaming to allow this synchronisation signal to be shared.
>> + */
>> + max9286_i2c_mux_open(priv);
>> +
>> + /* Start all cameras. */
>> + for_each_source(priv, source) {
>> + ret = v4l2_subdev_call(source->sd, video, s_stream, 1);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = max9286_check_video_links(priv);
>> + if (ret)
>> + return ret;
>> +
>> + /*
>> + * Wait until frame synchronization is locked.
>> + *
>> + * Manual says frame sync locking should take ~6 VTS.
>> + * From practical experience at least 8 are required. Give
>> + * 12 complete frames time (~400ms at 30 fps) to achieve frame
>> + * locking before returning error.
>> + */
>> + for (i = 0; i < 40; i++) {
>> + if (max9286_read(priv, 0x31) & MAX9286_FSYNC_LOCKED) {
>> + sync = true;
>> + break;
>> + }
>> + usleep_range(9000, 11000);
>> + }
>> +
>> + if (!sync) {
>> + dev_err(&priv->client->dev,
>> + "Failed to get frame synchronization\n");
>> + return -EXDEV; /* Invalid cross-device link */
>> + }
>> +
>> + /*
>> + * Enable CSI output, VC set according to link number.
>> + * Bit 7 must be set (chip manual says it's 0 and reserved).
>> + */
>> + max9286_write(priv, 0x15, 0x80 | MAX9286_VCTYPE |
>> + MAX9286_CSIOUTEN | MAX9286_0X15_RESV);
>> + } else {
>> + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV);
>> +
>> + /* Stop all cameras. */
>> + for_each_source(priv, source)
>> + v4l2_subdev_call(source->sd, video, s_stream, 0);
>> +
>> + max9286_i2c_mux_close(priv);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int max9286_enum_mbus_code(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_mbus_code_enum *code)
>> +{
>> + if (code->pad || code->index > 0)
>> + return -EINVAL;
>> +
>> + code->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> +
>> + return 0;
>> +}
>> +
>> +static struct v4l2_mbus_framefmt *
>> +max9286_get_pad_format(struct max9286_priv *priv,
>> + struct v4l2_subdev_pad_config *cfg,
>> + unsigned int pad, u32 which)
>> +{
>> + switch (which) {
>> + case V4L2_SUBDEV_FORMAT_TRY:
>> + return v4l2_subdev_get_try_format(&priv->sd, cfg, pad);
>> + case V4L2_SUBDEV_FORMAT_ACTIVE:
>> + return &priv->fmt[pad];
>> + default:
>> + return NULL;
>> + }
>> +}
>> +
>> +static int max9286_set_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *format)
>> +{
>> + struct max9286_priv *priv = sd_to_max9286(sd);
>> + struct v4l2_mbus_framefmt *cfg_fmt;
>> +
>> + if (format->pad >= MAX9286_SRC_PAD)
>> + return -EINVAL;
>> +
>> + /* Refuse non YUV422 formats as we hardcode DT to 8 bit YUV422 */
>> + switch (format->format.code) {
>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>> + case MEDIA_BUS_FMT_YUYV8_2X8:
>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>> + break;
>> + default:
>> + format->format.code = MEDIA_BUS_FMT_YUYV8_2X8;
>
> Is there any reason for not setting default format to MEDIA_BUS_FMT_UYVY8_2X8?
>
No good reason no, and I see that in max9286_enum_mbus_code(), we
currently code to MEDIA_BUS_FMT_YUYV8_2X8, and that's the value that we
init to, so that would be a better default indeed.
I'll update this for v10.
> Thanks,
> Mani
>
>> + break;
>> + }
>> +
>> + cfg_fmt = max9286_get_pad_format(priv, cfg, format->pad, format->which);
>> + if (!cfg_fmt)
>> + return -EINVAL;
>> +
>> + mutex_lock(&priv->mutex);
>> + *cfg_fmt = format->format;
>> + mutex_unlock(&priv->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static int max9286_get_fmt(struct v4l2_subdev *sd,
>> + struct v4l2_subdev_pad_config *cfg,
>> + struct v4l2_subdev_format *format)
>> +{
>> + struct max9286_priv *priv = sd_to_max9286(sd);
>> + struct v4l2_mbus_framefmt *cfg_fmt;
>> +
>> + if (format->pad >= MAX9286_SRC_PAD)
>> + return -EINVAL;
>> +
>> + cfg_fmt = max9286_get_pad_format(priv, cfg, format->pad, format->which);
>> + if (!cfg_fmt)
>> + return -EINVAL;
>> +
>> + mutex_lock(&priv->mutex);
>> + format->format = *cfg_fmt;
>> + mutex_unlock(&priv->mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_video_ops max9286_video_ops = {
>> + .s_stream = max9286_s_stream,
>> +};
>> +
>> +static const struct v4l2_subdev_pad_ops max9286_pad_ops = {
>> + .enum_mbus_code = max9286_enum_mbus_code,
>> + .get_fmt = max9286_get_fmt,
>> + .set_fmt = max9286_set_fmt,
>> +};
>> +
>> +static const struct v4l2_subdev_ops max9286_subdev_ops = {
>> + .video = &max9286_video_ops,
>> + .pad = &max9286_pad_ops,
>> +};
>> +
>> +static void max9286_init_format(struct v4l2_mbus_framefmt *fmt)
>> +{
>> + fmt->width = 1280;
>> + fmt->height = 800;
>> + fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
>> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
>> + fmt->field = V4L2_FIELD_NONE;
>> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
>> + fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
>> + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>> +}
>> +
>> +static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
>> +{
>> + struct v4l2_mbus_framefmt *format;
>> + unsigned int i;
>> +
>> + for (i = 0; i < MAX9286_N_SINKS; i++) {
>> + format = v4l2_subdev_get_try_format(subdev, fh->pad, i);
>> + max9286_init_format(format);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct v4l2_subdev_internal_ops max9286_subdev_internal_ops = {
>> + .open = max9286_open,
>> +};
>> +
>> +static int max9286_s_ctrl(struct v4l2_ctrl *ctrl)
>> +{
>> + switch (ctrl->id) {
>> + case V4L2_CID_PIXEL_RATE:
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static const struct v4l2_ctrl_ops max9286_ctrl_ops = {
>> + .s_ctrl = max9286_s_ctrl,
>> +};
>> +
>> +static int max9286_v4l2_register(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct fwnode_handle *ep;
>> + int ret;
>> + int i;
>> +
>> + /* Register v4l2 async notifiers for connected Camera subdevices */
>> + ret = max9286_v4l2_notifier_register(priv);
>> + if (ret) {
>> + dev_err(dev, "Unable to register V4L2 async notifiers\n");
>> + return ret;
>> + }
>> +
>> + /* Configure V4L2 for the MAX9286 itself */
>> +
>> + for (i = 0; i < MAX9286_N_SINKS; i++)
>> + max9286_init_format(&priv->fmt[i]);
>> +
>> + v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops);
>> + priv->sd.internal_ops = &max9286_subdev_internal_ops;
>> + priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>> +
>> + v4l2_ctrl_handler_init(&priv->ctrls, 1);
>> + priv->pixelrate = v4l2_ctrl_new_std(&priv->ctrls,
>> + &max9286_ctrl_ops,
>> + V4L2_CID_PIXEL_RATE,
>> + 1, INT_MAX, 1, 50000000);
>> +
>> + priv->sd.ctrl_handler = &priv->ctrls;
>> + ret = priv->ctrls.error;
>> + if (ret)
>> + goto err_async;
>> +
>> + priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
>> +
>> + priv->pads[MAX9286_SRC_PAD].flags = MEDIA_PAD_FL_SOURCE;
>> + for (i = 0; i < MAX9286_SRC_PAD; i++)
>> + priv->pads[i].flags = MEDIA_PAD_FL_SINK;
>> + ret = media_entity_pads_init(&priv->sd.entity, MAX9286_N_PADS,
>> + priv->pads);
>> + if (ret)
>> + goto err_async;
>> +
>> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), MAX9286_SRC_PAD,
>> + 0, 0);
>> + if (!ep) {
>> + dev_err(dev, "Unable to retrieve endpoint on \"port@4\"\n");
>> + ret = -ENOENT;
>> + goto err_async;
>> + }
>> + priv->sd.fwnode = ep;
>> +
>> + ret = v4l2_async_register_subdev(&priv->sd);
>> + if (ret < 0) {
>> + dev_err(dev, "Unable to register subdevice\n");
>> + goto err_put_node;
>> + }
>> +
>> + return 0;
>> +
>> +err_put_node:
>> + fwnode_handle_put(ep);
>> +err_async:
>> + max9286_v4l2_notifier_unregister(priv);
>> +
>> + return ret;
>> +}
>> +
>> +static void max9286_v4l2_unregister(struct max9286_priv *priv)
>> +{
>> + fwnode_handle_put(priv->sd.fwnode);
>> + v4l2_async_unregister_subdev(&priv->sd);
>> + max9286_v4l2_notifier_unregister(priv);
>> +}
>> +
>> +/* -----------------------------------------------------------------------------
>> + * Probe/Remove
>> + */
>> +
>> +static int max9286_setup(struct max9286_priv *priv)
>> +{
>> + /*
>> + * Link ordering values for all enabled links combinations. Orders must
>> + * be assigned sequentially from 0 to the number of enabled links
>> + * without leaving any hole for disabled links. We thus assign orders to
>> + * enabled links first, and use the remaining order values for disabled
>> + * links are all links must have a different order value;
>> + */
>> + static const u8 link_order[] = {
>> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xxxx */
>> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xxx0 */
>> + (3 << 6) | (2 << 4) | (0 << 2) | (1 << 0), /* xx0x */
>> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xx10 */
>> + (3 << 6) | (0 << 4) | (2 << 2) | (1 << 0), /* x0xx */
>> + (3 << 6) | (1 << 4) | (2 << 2) | (0 << 0), /* x1x0 */
>> + (3 << 6) | (1 << 4) | (0 << 2) | (2 << 0), /* x10x */
>> + (3 << 6) | (1 << 4) | (1 << 2) | (0 << 0), /* x210 */
>> + (0 << 6) | (3 << 4) | (2 << 2) | (1 << 0), /* 0xxx */
>> + (1 << 6) | (3 << 4) | (2 << 2) | (0 << 0), /* 1xx0 */
>> + (1 << 6) | (3 << 4) | (0 << 2) | (2 << 0), /* 1x0x */
>> + (2 << 6) | (3 << 4) | (1 << 2) | (0 << 0), /* 2x10 */
>> + (1 << 6) | (0 << 4) | (3 << 2) | (2 << 0), /* 10xx */
>> + (2 << 6) | (1 << 4) | (3 << 2) | (0 << 0), /* 21x0 */
>> + (2 << 6) | (1 << 4) | (0 << 2) | (3 << 0), /* 210x */
>> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* 3210 */
>> + };
>> +
>> + /*
>> + * Set the I2C bus speed.
>> + *
>> + * Enable I2C Local Acknowledge during the probe sequences of the camera
>> + * only. This should be disabled after the mux is initialised.
>> + */
>> + max9286_configure_i2c(priv, true);
>> +
>> + /*
>> + * Reverse channel setup.
>> + *
>> + * - Enable custom reverse channel configuration (through register 0x3f)
>> + * and set the first pulse length to 35 clock cycles.
>> + * - Increase the reverse channel amplitude to 170mV to accommodate the
>> + * high threshold enabled by the serializer driver.
>> + */
>> + max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
>> + max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
>> + MAX9286_REV_AMP_X);
>> + usleep_range(2000, 2500);
>> +
>> + /*
>> + * Enable GMSL links, mask unused ones and autodetect link
>> + * used as CSI clock source.
>> + */
>> + max9286_write(priv, 0x00, MAX9286_MSTLINKSEL_AUTO | priv->route_mask);
>> + max9286_write(priv, 0x0b, link_order[priv->route_mask]);
>> + max9286_write(priv, 0x69, (0xf & ~priv->route_mask));
>> +
>> + /*
>> + * Video format setup:
>> + * Disable CSI output, VC is set according to Link number.
>> + */
>> + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV);
>> +
>> + /* Enable CSI-2 Lane D0-D3 only, DBL mode, YUV422 8-bit. */
>> + max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL |
>> + MAX9286_CSILANECNT(priv->csi2_data_lanes) |
>> + MAX9286_DATATYPE_YUV422_8BIT);
>> +
>> + /* Automatic: FRAMESYNC taken from the slowest Link. */
>> + max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
>> + MAX9286_FSYNCMETH_AUTO);
>> +
>> + /* Enable HS/VS encoding, use D14/15 for HS/VS, invert VS. */
>> + max9286_write(priv, 0x0c, MAX9286_HVEN | MAX9286_INVVS |
>> + MAX9286_HVSRC_D14);
>> +
>> + /*
>> + * The overlap window seems to provide additional validation by tracking
>> + * the delay between vsync and frame sync, generating an error if the
>> + * delay is bigger than the programmed window, though it's not yet clear
>> + * what value should be set.
>> + *
>> + * As it's an optional value and can be disabled, we do so by setting
>> + * a 0 overlap value.
>> + */
>> + max9286_write(priv, 0x63, 0);
>> + max9286_write(priv, 0x64, 0);
>> +
>> + /*
>> + * Wait for 2ms to allow the link to resynchronize after the
>> + * configuration change.
>> + */
>> + usleep_range(2000, 5000);
>> +
>> + return 0;
>> +}
>> +
>> +static void max9286_gpio_set(struct gpio_chip *chip,
>> + unsigned int offset, int value)
>> +{
>> + struct max9286_priv *priv = gpiochip_get_data(chip);
>> +
>> + if (value)
>> + priv->gpio_state |= BIT(offset);
>> + else
>> + priv->gpio_state &= ~BIT(offset);
>> +
>> + max9286_write(priv, 0x0f, MAX9286_0X0F_RESERVED | priv->gpio_state);
>> +}
>> +
>> +static int max9286_gpio_get(struct gpio_chip *chip, unsigned int offset)
>> +{
>> + struct max9286_priv *priv = gpiochip_get_data(chip);
>> +
>> + return priv->gpio_state & BIT(offset);
>> +}
>> +
>> +static int max9286_register_gpio(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct gpio_chip *gpio = &priv->gpio;
>> + int ret;
>> +
>> + static const char * const names[] = {
>> + "GPIO0OUT",
>> + "GPIO1OUT",
>> + };
>> +
>> + /* Configure the GPIO */
>> + gpio->label = dev_name(dev);
>> + gpio->parent = dev;
>> + gpio->owner = THIS_MODULE;
>> + gpio->of_node = dev->of_node;
>> + gpio->ngpio = 2;
>> + gpio->base = -1;
>> + gpio->set = max9286_gpio_set;
>> + gpio->get = max9286_gpio_get;
>> + gpio->can_sleep = true;
>> + gpio->names = names;
>> +
>> + /* GPIO values default to high */
>> + priv->gpio_state = BIT(0) | BIT(1);
>> +
>> + ret = devm_gpiochip_add_data(dev, gpio, priv);
>> + if (ret)
>> + dev_err(dev, "Unable to create gpio_chip\n");
>> +
>> + return ret;
>> +}
>> +
>> +static int max9286_init(struct device *dev)
>> +{
>> + struct max9286_priv *priv;
>> + struct i2c_client *client;
>> + int ret;
>> +
>> + client = to_i2c_client(dev);
>> + priv = i2c_get_clientdata(client);
>> +
>> + /* Enable the bus power. */
>> + ret = regulator_enable(priv->regulator);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "Unable to turn PoC on\n");
>> + return ret;
>> + }
>> +
>> + ret = max9286_setup(priv);
>> + if (ret) {
>> + dev_err(dev, "Unable to setup max9286\n");
>> + goto err_regulator;
>> + }
>> +
>> + /*
>> + * Register all V4L2 interactions for the MAX9286 and notifiers for
>> + * any subdevices connected.
>> + */
>> + ret = max9286_v4l2_register(priv);
>> + if (ret) {
>> + dev_err(dev, "Failed to register with V4L2\n");
>> + goto err_regulator;
>> + }
>> +
>> + ret = max9286_i2c_mux_init(priv);
>> + if (ret) {
>> + dev_err(dev, "Unable to initialize I2C multiplexer\n");
>> + goto err_v4l2_register;
>> + }
>> +
>> + /* Leave the mux channels disabled until they are selected. */
>> + max9286_i2c_mux_close(priv);
>> +
>> + return 0;
>> +
>> +err_v4l2_register:
>> + max9286_v4l2_unregister(priv);
>> +err_regulator:
>> + regulator_disable(priv->regulator);
>> +
>> + return ret;
>> +}
>> +
>> +static void max9286_cleanup_dt(struct max9286_priv *priv)
>> +{
>> + struct max9286_source *source;
>> +
>> + for_each_source(priv, source) {
>> + fwnode_handle_put(source->fwnode);
>> + source->fwnode = NULL;
>> + }
>> +}
>> +
>> +static int max9286_parse_dt(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct device_node *i2c_mux;
>> + struct device_node *node = NULL;
>> + unsigned int i2c_mux_mask = 0;
>> +
>> + of_node_get(dev->of_node);
>> + i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
>> + if (!i2c_mux) {
>> + dev_err(dev, "Failed to find i2c-mux node\n");
>> + of_node_put(dev->of_node);
>> + return -EINVAL;
>> + }
>> +
>> + /* Identify which i2c-mux channels are enabled */
>> + for_each_child_of_node(i2c_mux, node) {
>> + u32 id = 0;
>> +
>> + of_property_read_u32(node, "reg", &id);
>> + if (id >= MAX9286_NUM_GMSL)
>> + continue;
>> +
>> + if (!of_device_is_available(node)) {
>> + dev_dbg(dev, "Skipping disabled I2C bus port %u\n", id);
>> + continue;
>> + }
>> +
>> + i2c_mux_mask |= BIT(id);
>> + }
>> + of_node_put(node);
>> + of_node_put(i2c_mux);
>> +
>> + /* Parse the endpoints */
>> + for_each_endpoint_of_node(dev->of_node, node) {
>> + struct max9286_source *source;
>> + struct of_endpoint ep;
>> +
>> + of_graph_parse_endpoint(node, &ep);
>> + dev_dbg(dev, "Endpoint %pOF on port %d",
>> + ep.local_node, ep.port);
>> +
>> + if (ep.port > MAX9286_NUM_GMSL) {
>> + dev_err(dev, "Invalid endpoint %s on port %d",
>> + of_node_full_name(ep.local_node), ep.port);
>> + continue;
>> + }
>> +
>> + /* For the source endpoint just parse the bus configuration. */
>> + if (ep.port == MAX9286_SRC_PAD) {
>> + struct v4l2_fwnode_endpoint vep = {
>> + .bus_type = V4L2_MBUS_CSI2_DPHY
>> + };
>> + int ret;
>> +
>> + ret = v4l2_fwnode_endpoint_parse(
>> + of_fwnode_handle(node), &vep);
>> + if (ret) {
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> + return ret;
>> + }
>> +
>> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
>> + dev_err(dev,
>> + "Media bus %u type not supported\n",
>> + vep.bus_type);
>> + v4l2_fwnode_endpoint_free(&vep);
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> + return -EINVAL;
>> + }
>> +
>> + priv->csi2_data_lanes =
>> + vep.bus.mipi_csi2.num_data_lanes;
>> + v4l2_fwnode_endpoint_free(&vep);
>> +
>> + continue;
>> + }
>> +
>> + /* Skip if the corresponding GMSL link is unavailable. */
>> + if (!(i2c_mux_mask & BIT(ep.port)))
>> + continue;
>> +
>> + if (priv->sources[ep.port].fwnode) {
>> + dev_err(dev,
>> + "Multiple port endpoints are not supported: %d",
>> + ep.port);
>> +
>> + continue;
>> + }
>> +
>> + source = &priv->sources[ep.port];
>> + source->fwnode = fwnode_graph_get_remote_endpoint(
>> + of_fwnode_handle(node));
>> + if (!source->fwnode) {
>> + dev_err(dev,
>> + "Endpoint %pOF has no remote endpoint connection\n",
>> + ep.local_node);
>> +
>> + continue;
>> + }
>> +
>> + priv->source_mask |= BIT(ep.port);
>> + priv->nsources++;
>> + }
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> +
>> + priv->route_mask = priv->source_mask;
>> +
>> + return 0;
>> +}
>> +
>> +static int max9286_probe(struct i2c_client *client)
>> +{
>> + struct max9286_priv *priv;
>> + int ret;
>> +
>> + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + mutex_init(&priv->mutex);
>> +
>> + priv->client = client;
>> + i2c_set_clientdata(client, priv);
>> +
>> + priv->gpiod_pwdn = devm_gpiod_get_optional(&client->dev, "enable",
>> + GPIOD_OUT_HIGH);
>> + if (IS_ERR(priv->gpiod_pwdn))
>> + return PTR_ERR(priv->gpiod_pwdn);
>> +
>> + gpiod_set_consumer_name(priv->gpiod_pwdn, "max9286-pwdn");
>> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 1);
>> +
>> + /* Wait at least 4ms before the I2C lines latch to the address */
>> + if (priv->gpiod_pwdn)
>> + usleep_range(4000, 5000);
>> +
>> + /*
>> + * The MAX9286 starts by default with all ports enabled, we disable all
>> + * ports early to ensure that all channels are disabled if we error out
>> + * and keep the bus consistent.
>> + */
>> + max9286_i2c_mux_close(priv);
>> +
>> + /*
>> + * The MAX9286 initialises with auto-acknowledge enabled by default.
>> + * This can be invasive to other transactions on the same bus, so
>> + * disable it early. It will be enabled only as and when needed.
>> + */
>> + max9286_configure_i2c(priv, false);
>> +
>> + ret = max9286_register_gpio(priv);
>> + if (ret)
>> + goto err_powerdown;
>> +
>> + priv->regulator = devm_regulator_get(&client->dev, "poc");
>> + if (IS_ERR(priv->regulator)) {
>> + if (PTR_ERR(priv->regulator) != -EPROBE_DEFER)
>> + dev_err(&client->dev,
>> + "Unable to get PoC regulator (%ld)\n",
>> + PTR_ERR(priv->regulator));
>> + ret = PTR_ERR(priv->regulator);
>> + goto err_powerdown;
>> + }
>> +
>> + ret = max9286_parse_dt(priv);
>> + if (ret)
>> + goto err_powerdown;
>> +
>> + ret = max9286_init(&client->dev);
>> + if (ret < 0)
>> + goto err_cleanup_dt;
>> +
>> + return 0;
>> +
>> +err_cleanup_dt:
>> + max9286_cleanup_dt(priv);
>> +err_powerdown:
>> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
>> +
>> + return ret;
>> +}
>> +
>> +static int max9286_remove(struct i2c_client *client)
>> +{
>> + struct max9286_priv *priv = i2c_get_clientdata(client);
>> +
>> + i2c_mux_del_adapters(priv->mux);
>> +
>> + max9286_v4l2_unregister(priv);
>> +
>> + regulator_disable(priv->regulator);
>> +
>> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
>> +
>> + max9286_cleanup_dt(priv);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id max9286_dt_ids[] = {
>> + { .compatible = "maxim,max9286" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, max9286_dt_ids);
>> +
>> +static struct i2c_driver max9286_i2c_driver = {
>> + .driver = {
>> + .name = "max9286",
>> + .of_match_table = of_match_ptr(max9286_dt_ids),
>> + },
>> + .probe_new = max9286_probe,
>> + .remove = max9286_remove,
>> +};
>> +
>> +module_i2c_driver(max9286_i2c_driver);
>> +
>> +MODULE_DESCRIPTION("Maxim MAX9286 GMSL Deserializer Driver");
>> +MODULE_AUTHOR("Jacopo Mondi, Kieran Bingham, Laurent Pinchart, Niklas Söderlund, Vladimir Barinov");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.25.1
>>
^ permalink raw reply
* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Manivannan Sadhasivam @ 2020-05-14 9:59 UTC (permalink / raw)
To: Kieran Bingham
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Rob Herring, Jacopo Mondi, Laurent Pinchart,
Niklas Söderlund
In-Reply-To: <e07af762-2db1-920c-5ab8-5528bdcfc2ce@ideasonboard.com>
On Thu, May 14, 2020 at 10:53:00AM +0100, Kieran Bingham wrote:
> Hi Mani,
>
> On 12/05/2020 18:40, Manivannan Sadhasivam wrote:
> > Hi Kieran,
> >
> > On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:
> >> The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
> >> CSI-2 output. The device supports multicamera streaming applications,
> >> and features the ability to synchronise the attached cameras.
> >>
> >> CSI-2 output can be configured with 1 to 4 lanes, and a control channel
> >> is supported over I2C, which implements an I2C mux to facilitate
> >> communications with connected cameras across the reverse control
> >> channel.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>
> >> --
> >> v2:
> >> - Fix MAINTAINERS entry
> >>
> >> This posting is released with the following modifications to work
> >> without Sakari's VC developments:
> >> - max9286_g_mbus_config() re-instated
> >> - max9286_get_frame_desc() is not bus/csi aware
> >> - max9286_{get,set}_routing() removed
> >>
> >> v3:
> >> - Initialise notifier with v4l2_async_notifier_init
> >> - Update for new mbus csi2 format V4L2_MBUS_CSI2_DPHY
> >>
> >> v4: - Re-introduce required code to function with the VC series.
> >>
> >> - Implement max9286_get_routing, max9286_set_routing
> >> - Remove max9286_g_mbus_config
> >>
> >> v5: (internal release)
> >> - Fix printk formatting for hex value
> >> - max9286->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE (add |)
> >> - MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER -> MEDIA_ENT_F_VID_IF_BRIDGE
> >> - Remove 'device is bound' workaround
> >>
> >> v6:
> >> - v4l2_subdev_krouting instead of v4l2_subdev_routing separated
> >> to allow integration without the VC/V4L2-Mux series.
> >> - convert sd_to_max9286 to inline function
> >> - rename max9286_device to max9286_priv
> >> - Cleanup the v4l2_async_notifier
> >> - Extend MODULE_AUTHOR
> >> - Replace of_graph_get_endpoint_by_regs with fwnode_graph_get_endpoint_by_id
> >> - Pass default bus type when parsing fwnode endpoint (Manivannan Sadhasivam)
> >> - Use new YAML file reference in MAINTAINERS
> >> - Parse new i2c-mux node in max9286_get_i2c_by_id
> >> (This could/should be refactored to parse these separately first)
> >> - Spelling and calculation fixes in the FSYNC_LOCKED check comments
> >> - Identify each enabled i2c-mux channel in a single pass
> >> - max9286: Improve mux-state readbility [v2]
> >> - Fix frame sync lock durations
> >> - Add comment to describe /why/ we must open the mux in s_stream
> >> - use -EXDEV as return code for failed link synchronisation.
> >> - Fix reference counting of the dt nodeS
> >> - Convert to probe_new for I2C
> >> - Remove redundant max9286_i2c_mux_state
> >> - Provide optional enable-gpio (max9286-pwdn)
> >>
> >> v7:
> >> [Kieran]
> >> - Ensure powerdown lines are optional
> >> - Add a 4ms power-up delay
> >> - Add max9286_check_config_link() to core
> >> - Add GPIO chip controller for GPIO0OUT and GPIO1OUT
> >> - Fix GPIO registration
> >> - max9286: Split out async registration
> >> (fixes regulator -EPROBE_DEFERs failures)
> >> - Collect all V4L2 registrations
> >> - balance v4l2_async refcnting
> >> - Rename max9286_v4l2_async_ => max9286_v4l2_notifier_
> >>
> >> [Jacopo]
> >> - Remove redundanct MAXIM_I2C_SPEED macros
> >> - Move notifiers operations
> >> - Add delay after reverse channel reconfiguration
> >> - Move link setup to completion
> >> - Fix up max9286_check_config_link() implementation
> >> - Remove redundant dual configuration of reverse channel
> >>
> >> v8:
> >>
> >> [Kieran]
> >> - Update the bound_sources mask on unbind
> >> - Convert probe kzalloc usage to devm_ variant
> >> - Fix up cleanup path from GPIO PowerDown registration
> >> - cleanup GPIO device registration fail path
> >> - Convert to use devm_regulator_get()
> >> - Fit max9286_parse_dt print on one line
> >> - Move multi-device workarounds out of upstream driver
> >> - Remove I2C mod-table
> >> - Lock format changes
> >> - Describe pad index usage
> >> - Remove poc_enabled workaround
> >> - Rename the max9286_gpio to be more explicit on it's actions.
> >> - Move max9286_init_format call
> >> - Rework probe sequence and simplify error paths.
> >> - Simplify i2c comments
> >> - Implement Pixelrate control
> >> - Disable overlap window
> >>
> >> [Jacopo]
> >> - Adapt Kconfig to latest upstream changes
> >> - Put of node on error
> >> - Calculate pixel rate
> >> - Simplify overlap window disablement
> >>
> >> v9:
> >>
> >> [Kieran]
> >> - Kconfig: Depend on OF
> >> - Re-sort addition to Makefile
> >> ---
> >> MAINTAINERS | 10 +
> >> drivers/media/i2c/Kconfig | 13 +
> >> drivers/media/i2c/Makefile | 1 +
> >> drivers/media/i2c/max9286.c | 1332 +++++++++++++++++++++++++++++++++++
> >> 4 files changed, 1356 insertions(+)
> >> create mode 100644 drivers/media/i2c/max9286.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index a7bb6e22d5da..99e3bf7760fd 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -10274,6 +10274,16 @@ F: Documentation/hwmon/max6697.rst
> >> F: drivers/hwmon/max6697.c
> >> F: include/linux/platform_data/max6697.h
> >>
> >> +MAX9286 QUAD GMSL DESERIALIZER DRIVER
> >> +M: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> +M: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> +M: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >> +M: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> +L: linux-media@vger.kernel.org
> >> +S: Maintained
> >> +F: Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >> +F: drivers/media/i2c/max9286.c
> >> +
> >> MAX9860 MONO AUDIO VOICE CODEC DRIVER
> >> M: Peter Rosin <peda@axentia.se>
> >> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> >> index 3abc80373ec0..2e390f41f6da 100644
> >> --- a/drivers/media/i2c/Kconfig
> >> +++ b/drivers/media/i2c/Kconfig
> >> @@ -464,6 +464,19 @@ config VIDEO_VPX3220
> >> To compile this driver as a module, choose M here: the
> >> module will be called vpx3220.
> >>
> >> +config VIDEO_MAX9286
> >> + tristate "Maxim MAX9286 GMSL deserializer support"
> >> + depends on I2C && I2C_MUX
> >> + depends on OF
> >> + select V4L2_FWNODE
> >> + select VIDEO_V4L2_SUBDEV_API
> >> + select MEDIA_CONTROLLER
> >> + help
> >> + This driver supports the Maxim MAX9286 GMSL deserializer.
> >> +
> >> + To compile this driver as a module, choose M here: the
> >> + module will be called max9286.
> >> +
> >> comment "Video and audio decoders"
> >>
> >> config VIDEO_SAA717X
> >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> >> index 77bf7d0b691f..f0b001ee4b05 100644
> >> --- a/drivers/media/i2c/Makefile
> >> +++ b/drivers/media/i2c/Makefile
> >> @@ -117,6 +117,7 @@ obj-$(CONFIG_VIDEO_IMX274) += imx274.o
> >> obj-$(CONFIG_VIDEO_IMX290) += imx290.o
> >> obj-$(CONFIG_VIDEO_IMX319) += imx319.o
> >> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> >> +obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
> >> obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> >>
> >> obj-$(CONFIG_SDR_MAX2175) += max2175.o
> >> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> >> new file mode 100644
> >> index 000000000000..481d65f2b51d
> >> --- /dev/null
> >> +++ b/drivers/media/i2c/max9286.c
> >> @@ -0,0 +1,1332 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Maxim MAX9286 GMSL Deserializer Driver
> >> + *
> >> + * Copyright (C) 2017-2019 Jacopo Mondi
> >> + * Copyright (C) 2017-2019 Kieran Bingham
> >> + * Copyright (C) 2017-2019 Laurent Pinchart
> >> + * Copyright (C) 2017-2019 Niklas Söderlund
> >> + * Copyright (C) 2016 Renesas Electronics Corporation
> >> + * Copyright (C) 2015 Cogent Embedded, Inc.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/device.h>
> >> +#include <linux/fwnode.h>
> >> +#include <linux/gpio/consumer.h>
> >> +#include <linux/gpio/driver.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/i2c-mux.h>
> >> +#include <linux/module.h>
> >> +#include <linux/mutex.h>
> >> +#include <linux/of_graph.h>
> >> +#include <linux/regulator/consumer.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#include <media/v4l2-async.h>
> >> +#include <media/v4l2-ctrls.h>
> >> +#include <media/v4l2-device.h>
> >> +#include <media/v4l2-fwnode.h>
> >> +#include <media/v4l2-subdev.h>
> >> +
> >> +/* Register 0x00 */
> >> +#define MAX9286_MSTLINKSEL_AUTO (7 << 5)
> >> +#define MAX9286_MSTLINKSEL(n) ((n) << 5)
> >> +#define MAX9286_EN_VS_GEN BIT(4)
> >> +#define MAX9286_LINKEN(n) (1 << (n))
> >> +/* Register 0x01 */
> >> +#define MAX9286_FSYNCMODE_ECU (3 << 6)
> >> +#define MAX9286_FSYNCMODE_EXT (2 << 6)
> >> +#define MAX9286_FSYNCMODE_INT_OUT (1 << 6)
> >> +#define MAX9286_FSYNCMODE_INT_HIZ (0 << 6)
> >> +#define MAX9286_GPIEN BIT(5)
> >> +#define MAX9286_ENLMO_RSTFSYNC BIT(2)
> >> +#define MAX9286_FSYNCMETH_AUTO (2 << 0)
> >> +#define MAX9286_FSYNCMETH_SEMI_AUTO (1 << 0)
> >> +#define MAX9286_FSYNCMETH_MANUAL (0 << 0)
> >> +#define MAX9286_REG_FSYNC_PERIOD_L 0x06
> >> +#define MAX9286_REG_FSYNC_PERIOD_M 0x07
> >> +#define MAX9286_REG_FSYNC_PERIOD_H 0x08
> >> +/* Register 0x0a */
> >> +#define MAX9286_FWDCCEN(n) (1 << ((n) + 4))
> >> +#define MAX9286_REVCCEN(n) (1 << (n))
> >> +/* Register 0x0c */
> >> +#define MAX9286_HVEN BIT(7)
> >> +#define MAX9286_EDC_6BIT_HAMMING (2 << 5)
> >> +#define MAX9286_EDC_6BIT_CRC (1 << 5)
> >> +#define MAX9286_EDC_1BIT_PARITY (0 << 5)
> >> +#define MAX9286_DESEL BIT(4)
> >> +#define MAX9286_INVVS BIT(3)
> >> +#define MAX9286_INVHS BIT(2)
> >> +#define MAX9286_HVSRC_D0 (2 << 0)
> >> +#define MAX9286_HVSRC_D14 (1 << 0)
> >> +#define MAX9286_HVSRC_D18 (0 << 0)
> >> +/* Register 0x0f */
> >> +#define MAX9286_0X0F_RESERVED BIT(3)
> >> +/* Register 0x12 */
> >> +#define MAX9286_CSILANECNT(n) (((n) - 1) << 6)
> >> +#define MAX9286_CSIDBL BIT(5)
> >> +#define MAX9286_DBL BIT(4)
> >> +#define MAX9286_DATATYPE_USER_8BIT (11 << 0)
> >> +#define MAX9286_DATATYPE_USER_YUV_12BIT (10 << 0)
> >> +#define MAX9286_DATATYPE_USER_24BIT (9 << 0)
> >> +#define MAX9286_DATATYPE_RAW14 (8 << 0)
> >> +#define MAX9286_DATATYPE_RAW11 (7 << 0)
> >> +#define MAX9286_DATATYPE_RAW10 (6 << 0)
> >> +#define MAX9286_DATATYPE_RAW8 (5 << 0)
> >> +#define MAX9286_DATATYPE_YUV422_10BIT (4 << 0)
> >> +#define MAX9286_DATATYPE_YUV422_8BIT (3 << 0)
> >> +#define MAX9286_DATATYPE_RGB555 (2 << 0)
> >> +#define MAX9286_DATATYPE_RGB565 (1 << 0)
> >> +#define MAX9286_DATATYPE_RGB888 (0 << 0)
> >> +/* Register 0x15 */
> >> +#define MAX9286_VC(n) ((n) << 5)
> >> +#define MAX9286_VCTYPE BIT(4)
> >> +#define MAX9286_CSIOUTEN BIT(3)
> >> +#define MAX9286_0X15_RESV (3 << 0)
> >> +/* Register 0x1b */
> >> +#define MAX9286_SWITCHIN(n) (1 << ((n) + 4))
> >> +#define MAX9286_ENEQ(n) (1 << (n))
> >> +/* Register 0x27 */
> >> +#define MAX9286_LOCKED BIT(7)
> >> +/* Register 0x31 */
> >> +#define MAX9286_FSYNC_LOCKED BIT(6)
> >> +/* Register 0x34 */
> >> +#define MAX9286_I2CLOCACK BIT(7)
> >> +#define MAX9286_I2CSLVSH_1046NS_469NS (3 << 5)
> >> +#define MAX9286_I2CSLVSH_938NS_352NS (2 << 5)
> >> +#define MAX9286_I2CSLVSH_469NS_234NS (1 << 5)
> >> +#define MAX9286_I2CSLVSH_352NS_117NS (0 << 5)
> >> +#define MAX9286_I2CMSTBT_837KBPS (7 << 2)
> >> +#define MAX9286_I2CMSTBT_533KBPS (6 << 2)
> >> +#define MAX9286_I2CMSTBT_339KBPS (5 << 2)
> >> +#define MAX9286_I2CMSTBT_173KBPS (4 << 2)
> >> +#define MAX9286_I2CMSTBT_105KBPS (3 << 2)
> >> +#define MAX9286_I2CMSTBT_84KBPS (2 << 2)
> >> +#define MAX9286_I2CMSTBT_28KBPS (1 << 2)
> >> +#define MAX9286_I2CMSTBT_8KBPS (0 << 2)
> >> +#define MAX9286_I2CSLVTO_NONE (3 << 0)
> >> +#define MAX9286_I2CSLVTO_1024US (2 << 0)
> >> +#define MAX9286_I2CSLVTO_256US (1 << 0)
> >> +#define MAX9286_I2CSLVTO_64US (0 << 0)
> >> +/* Register 0x3b */
> >> +#define MAX9286_REV_TRF(n) ((n) << 4)
> >> +#define MAX9286_REV_AMP(n) ((((n) - 30) / 10) << 1) /* in mV */
> >> +#define MAX9286_REV_AMP_X BIT(0)
> >> +/* Register 0x3f */
> >> +#define MAX9286_EN_REV_CFG BIT(6)
> >> +#define MAX9286_REV_FLEN(n) ((n) - 20)
> >> +/* Register 0x49 */
> >> +#define MAX9286_VIDEO_DETECT_MASK 0x0f
> >> +/* Register 0x69 */
> >> +#define MAX9286_LFLTBMONMASKED BIT(7)
> >> +#define MAX9286_LOCKMONMASKED BIT(6)
> >> +#define MAX9286_AUTOCOMBACKEN BIT(5)
> >> +#define MAX9286_AUTOMASKEN BIT(4)
> >> +#define MAX9286_MASKLINK(n) ((n) << 0)
> >> +
> >> +/*
> >> + * The sink and source pads are created to match the OF graph port numbers so
> >> + * that their indexes can be used interchangeably.
> >> + */
> >> +#define MAX9286_NUM_GMSL 4
> >> +#define MAX9286_N_SINKS 4
> >> +#define MAX9286_N_PADS 5
> >> +#define MAX9286_SRC_PAD 4
> >> +
> >> +struct max9286_source {
> >> + struct v4l2_async_subdev asd;
> >> + struct v4l2_subdev *sd;
> >> + struct fwnode_handle *fwnode;
> >> +};
> >> +
> >> +#define asd_to_max9286_source(_asd) \
> >> + container_of(_asd, struct max9286_source, asd)
> >> +
> >> +struct max9286_priv {
> >> + struct i2c_client *client;
> >> + struct gpio_desc *gpiod_pwdn;
> >> + struct v4l2_subdev sd;
> >> + struct media_pad pads[MAX9286_N_PADS];
> >> + struct regulator *regulator;
> >> +
> >> + struct gpio_chip gpio;
> >> + u8 gpio_state;
> >> +
> >> + struct i2c_mux_core *mux;
> >> + unsigned int mux_channel;
> >> + bool mux_open;
> >> +
> >> + struct v4l2_ctrl_handler ctrls;
> >> + struct v4l2_ctrl *pixelrate;
> >> +
> >> + struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
> >> +
> >> + /* Protects controls and fmt structures */
> >> + struct mutex mutex;
> >> +
> >> + unsigned int nsources;
> >> + unsigned int source_mask;
> >> + unsigned int route_mask;
> >> + unsigned int bound_sources;
> >> + unsigned int csi2_data_lanes;
> >> + struct max9286_source sources[MAX9286_NUM_GMSL];
> >> + struct v4l2_async_notifier notifier;
> >> +};
> >> +
[...]
> >> +static int max9286_enum_mbus_code(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_mbus_code_enum *code)
> >> +{
> >> + if (code->pad || code->index > 0)
> >> + return -EINVAL;
> >> +
> >> + code->code = MEDIA_BUS_FMT_UYVY8_2X8;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static struct v4l2_mbus_framefmt *
> >> +max9286_get_pad_format(struct max9286_priv *priv,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + unsigned int pad, u32 which)
> >> +{
> >> + switch (which) {
> >> + case V4L2_SUBDEV_FORMAT_TRY:
> >> + return v4l2_subdev_get_try_format(&priv->sd, cfg, pad);
> >> + case V4L2_SUBDEV_FORMAT_ACTIVE:
> >> + return &priv->fmt[pad];
> >> + default:
> >> + return NULL;
> >> + }
> >> +}
> >> +
> >> +static int max9286_set_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_format *format)
> >> +{
> >> + struct max9286_priv *priv = sd_to_max9286(sd);
> >> + struct v4l2_mbus_framefmt *cfg_fmt;
> >> +
> >> + if (format->pad >= MAX9286_SRC_PAD)
> >> + return -EINVAL;
> >> +
> >> + /* Refuse non YUV422 formats as we hardcode DT to 8 bit YUV422 */
> >> + switch (format->format.code) {
> >> + case MEDIA_BUS_FMT_UYVY8_2X8:
> >> + case MEDIA_BUS_FMT_VYUY8_2X8:
> >> + case MEDIA_BUS_FMT_YUYV8_2X8:
> >> + case MEDIA_BUS_FMT_YVYU8_2X8:
> >> + break;
> >> + default:
> >> + format->format.code = MEDIA_BUS_FMT_YUYV8_2X8;
> >
> > Is there any reason for not setting default format to MEDIA_BUS_FMT_UYVY8_2X8?
> >
>
>
> No good reason no, and I see that in max9286_enum_mbus_code(), we
> currently code to MEDIA_BUS_FMT_YUYV8_2X8, and that's the value that we
> init to, so that would be a better default indeed.
>
max9286_enum_mbus_code() returns MEDIA_BUS_FMT_UYVY8_2X8, no?
Thanks,
Mani
> I'll update this for v10.
>
>
> > Thanks,
> > Mani
> >
> >> + break;
> >> + }
> >> +
> >> + cfg_fmt = max9286_get_pad_format(priv, cfg, format->pad, format->which);
> >> + if (!cfg_fmt)
> >> + return -EINVAL;
> >> +
> >> + mutex_lock(&priv->mutex);
> >> + *cfg_fmt = format->format;
> >> + mutex_unlock(&priv->mutex);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int max9286_get_fmt(struct v4l2_subdev *sd,
> >> + struct v4l2_subdev_pad_config *cfg,
> >> + struct v4l2_subdev_format *format)
> >> +{
> >> + struct max9286_priv *priv = sd_to_max9286(sd);
> >> + struct v4l2_mbus_framefmt *cfg_fmt;
> >> +
> >> + if (format->pad >= MAX9286_SRC_PAD)
> >> + return -EINVAL;
> >> +
> >> + cfg_fmt = max9286_get_pad_format(priv, cfg, format->pad, format->which);
> >> + if (!cfg_fmt)
> >> + return -EINVAL;
> >> +
> >> + mutex_lock(&priv->mutex);
> >> + format->format = *cfg_fmt;
> >> + mutex_unlock(&priv->mutex);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct v4l2_subdev_video_ops max9286_video_ops = {
> >> + .s_stream = max9286_s_stream,
> >> +};
> >> +
> >> +static const struct v4l2_subdev_pad_ops max9286_pad_ops = {
> >> + .enum_mbus_code = max9286_enum_mbus_code,
> >> + .get_fmt = max9286_get_fmt,
> >> + .set_fmt = max9286_set_fmt,
> >> +};
> >> +
> >> +static const struct v4l2_subdev_ops max9286_subdev_ops = {
> >> + .video = &max9286_video_ops,
> >> + .pad = &max9286_pad_ops,
> >> +};
> >> +
> >> +static void max9286_init_format(struct v4l2_mbus_framefmt *fmt)
> >> +{
> >> + fmt->width = 1280;
> >> + fmt->height = 800;
> >> + fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
> >> + fmt->colorspace = V4L2_COLORSPACE_SRGB;
> >> + fmt->field = V4L2_FIELD_NONE;
> >> + fmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> >> + fmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> >> + fmt->xfer_func = V4L2_XFER_FUNC_DEFAULT;
> >> +}
> >> +
> >> +static int max9286_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh)
> >> +{
> >> + struct v4l2_mbus_framefmt *format;
> >> + unsigned int i;
> >> +
> >> + for (i = 0; i < MAX9286_N_SINKS; i++) {
> >> + format = v4l2_subdev_get_try_format(subdev, fh->pad, i);
> >> + max9286_init_format(format);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct v4l2_subdev_internal_ops max9286_subdev_internal_ops = {
> >> + .open = max9286_open,
> >> +};
> >> +
> >> +static int max9286_s_ctrl(struct v4l2_ctrl *ctrl)
> >> +{
> >> + switch (ctrl->id) {
> >> + case V4L2_CID_PIXEL_RATE:
> >> + return 0;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +}
> >> +
> >> +static const struct v4l2_ctrl_ops max9286_ctrl_ops = {
> >> + .s_ctrl = max9286_s_ctrl,
> >> +};
> >> +
> >> +static int max9286_v4l2_register(struct max9286_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct fwnode_handle *ep;
> >> + int ret;
> >> + int i;
> >> +
> >> + /* Register v4l2 async notifiers for connected Camera subdevices */
> >> + ret = max9286_v4l2_notifier_register(priv);
> >> + if (ret) {
> >> + dev_err(dev, "Unable to register V4L2 async notifiers\n");
> >> + return ret;
> >> + }
> >> +
> >> + /* Configure V4L2 for the MAX9286 itself */
> >> +
> >> + for (i = 0; i < MAX9286_N_SINKS; i++)
> >> + max9286_init_format(&priv->fmt[i]);
> >> +
> >> + v4l2_i2c_subdev_init(&priv->sd, priv->client, &max9286_subdev_ops);
> >> + priv->sd.internal_ops = &max9286_subdev_internal_ops;
> >> + priv->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >> +
> >> + v4l2_ctrl_handler_init(&priv->ctrls, 1);
> >> + priv->pixelrate = v4l2_ctrl_new_std(&priv->ctrls,
> >> + &max9286_ctrl_ops,
> >> + V4L2_CID_PIXEL_RATE,
> >> + 1, INT_MAX, 1, 50000000);
> >> +
> >> + priv->sd.ctrl_handler = &priv->ctrls;
> >> + ret = priv->ctrls.error;
> >> + if (ret)
> >> + goto err_async;
> >> +
> >> + priv->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
> >> +
> >> + priv->pads[MAX9286_SRC_PAD].flags = MEDIA_PAD_FL_SOURCE;
> >> + for (i = 0; i < MAX9286_SRC_PAD; i++)
> >> + priv->pads[i].flags = MEDIA_PAD_FL_SINK;
> >> + ret = media_entity_pads_init(&priv->sd.entity, MAX9286_N_PADS,
> >> + priv->pads);
> >> + if (ret)
> >> + goto err_async;
> >> +
> >> + ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), MAX9286_SRC_PAD,
> >> + 0, 0);
> >> + if (!ep) {
> >> + dev_err(dev, "Unable to retrieve endpoint on \"port@4\"\n");
> >> + ret = -ENOENT;
> >> + goto err_async;
> >> + }
> >> + priv->sd.fwnode = ep;
> >> +
> >> + ret = v4l2_async_register_subdev(&priv->sd);
> >> + if (ret < 0) {
> >> + dev_err(dev, "Unable to register subdevice\n");
> >> + goto err_put_node;
> >> + }
> >> +
> >> + return 0;
> >> +
> >> +err_put_node:
> >> + fwnode_handle_put(ep);
> >> +err_async:
> >> + max9286_v4l2_notifier_unregister(priv);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void max9286_v4l2_unregister(struct max9286_priv *priv)
> >> +{
> >> + fwnode_handle_put(priv->sd.fwnode);
> >> + v4l2_async_unregister_subdev(&priv->sd);
> >> + max9286_v4l2_notifier_unregister(priv);
> >> +}
> >> +
> >> +/* -----------------------------------------------------------------------------
> >> + * Probe/Remove
> >> + */
> >> +
> >> +static int max9286_setup(struct max9286_priv *priv)
> >> +{
> >> + /*
> >> + * Link ordering values for all enabled links combinations. Orders must
> >> + * be assigned sequentially from 0 to the number of enabled links
> >> + * without leaving any hole for disabled links. We thus assign orders to
> >> + * enabled links first, and use the remaining order values for disabled
> >> + * links are all links must have a different order value;
> >> + */
> >> + static const u8 link_order[] = {
> >> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xxxx */
> >> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xxx0 */
> >> + (3 << 6) | (2 << 4) | (0 << 2) | (1 << 0), /* xx0x */
> >> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* xx10 */
> >> + (3 << 6) | (0 << 4) | (2 << 2) | (1 << 0), /* x0xx */
> >> + (3 << 6) | (1 << 4) | (2 << 2) | (0 << 0), /* x1x0 */
> >> + (3 << 6) | (1 << 4) | (0 << 2) | (2 << 0), /* x10x */
> >> + (3 << 6) | (1 << 4) | (1 << 2) | (0 << 0), /* x210 */
> >> + (0 << 6) | (3 << 4) | (2 << 2) | (1 << 0), /* 0xxx */
> >> + (1 << 6) | (3 << 4) | (2 << 2) | (0 << 0), /* 1xx0 */
> >> + (1 << 6) | (3 << 4) | (0 << 2) | (2 << 0), /* 1x0x */
> >> + (2 << 6) | (3 << 4) | (1 << 2) | (0 << 0), /* 2x10 */
> >> + (1 << 6) | (0 << 4) | (3 << 2) | (2 << 0), /* 10xx */
> >> + (2 << 6) | (1 << 4) | (3 << 2) | (0 << 0), /* 21x0 */
> >> + (2 << 6) | (1 << 4) | (0 << 2) | (3 << 0), /* 210x */
> >> + (3 << 6) | (2 << 4) | (1 << 2) | (0 << 0), /* 3210 */
> >> + };
> >> +
> >> + /*
> >> + * Set the I2C bus speed.
> >> + *
> >> + * Enable I2C Local Acknowledge during the probe sequences of the camera
> >> + * only. This should be disabled after the mux is initialised.
> >> + */
> >> + max9286_configure_i2c(priv, true);
> >> +
> >> + /*
> >> + * Reverse channel setup.
> >> + *
> >> + * - Enable custom reverse channel configuration (through register 0x3f)
> >> + * and set the first pulse length to 35 clock cycles.
> >> + * - Increase the reverse channel amplitude to 170mV to accommodate the
> >> + * high threshold enabled by the serializer driver.
> >> + */
> >> + max9286_write(priv, 0x3f, MAX9286_EN_REV_CFG | MAX9286_REV_FLEN(35));
> >> + max9286_write(priv, 0x3b, MAX9286_REV_TRF(1) | MAX9286_REV_AMP(70) |
> >> + MAX9286_REV_AMP_X);
> >> + usleep_range(2000, 2500);
> >> +
> >> + /*
> >> + * Enable GMSL links, mask unused ones and autodetect link
> >> + * used as CSI clock source.
> >> + */
> >> + max9286_write(priv, 0x00, MAX9286_MSTLINKSEL_AUTO | priv->route_mask);
> >> + max9286_write(priv, 0x0b, link_order[priv->route_mask]);
> >> + max9286_write(priv, 0x69, (0xf & ~priv->route_mask));
> >> +
> >> + /*
> >> + * Video format setup:
> >> + * Disable CSI output, VC is set according to Link number.
> >> + */
> >> + max9286_write(priv, 0x15, MAX9286_VCTYPE | MAX9286_0X15_RESV);
> >> +
> >> + /* Enable CSI-2 Lane D0-D3 only, DBL mode, YUV422 8-bit. */
> >> + max9286_write(priv, 0x12, MAX9286_CSIDBL | MAX9286_DBL |
> >> + MAX9286_CSILANECNT(priv->csi2_data_lanes) |
> >> + MAX9286_DATATYPE_YUV422_8BIT);
> >> +
> >> + /* Automatic: FRAMESYNC taken from the slowest Link. */
> >> + max9286_write(priv, 0x01, MAX9286_FSYNCMODE_INT_HIZ |
> >> + MAX9286_FSYNCMETH_AUTO);
> >> +
> >> + /* Enable HS/VS encoding, use D14/15 for HS/VS, invert VS. */
> >> + max9286_write(priv, 0x0c, MAX9286_HVEN | MAX9286_INVVS |
> >> + MAX9286_HVSRC_D14);
> >> +
> >> + /*
> >> + * The overlap window seems to provide additional validation by tracking
> >> + * the delay between vsync and frame sync, generating an error if the
> >> + * delay is bigger than the programmed window, though it's not yet clear
> >> + * what value should be set.
> >> + *
> >> + * As it's an optional value and can be disabled, we do so by setting
> >> + * a 0 overlap value.
> >> + */
> >> + max9286_write(priv, 0x63, 0);
> >> + max9286_write(priv, 0x64, 0);
> >> +
> >> + /*
> >> + * Wait for 2ms to allow the link to resynchronize after the
> >> + * configuration change.
> >> + */
> >> + usleep_range(2000, 5000);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void max9286_gpio_set(struct gpio_chip *chip,
> >> + unsigned int offset, int value)
> >> +{
> >> + struct max9286_priv *priv = gpiochip_get_data(chip);
> >> +
> >> + if (value)
> >> + priv->gpio_state |= BIT(offset);
> >> + else
> >> + priv->gpio_state &= ~BIT(offset);
> >> +
> >> + max9286_write(priv, 0x0f, MAX9286_0X0F_RESERVED | priv->gpio_state);
> >> +}
> >> +
> >> +static int max9286_gpio_get(struct gpio_chip *chip, unsigned int offset)
> >> +{
> >> + struct max9286_priv *priv = gpiochip_get_data(chip);
> >> +
> >> + return priv->gpio_state & BIT(offset);
> >> +}
> >> +
> >> +static int max9286_register_gpio(struct max9286_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct gpio_chip *gpio = &priv->gpio;
> >> + int ret;
> >> +
> >> + static const char * const names[] = {
> >> + "GPIO0OUT",
> >> + "GPIO1OUT",
> >> + };
> >> +
> >> + /* Configure the GPIO */
> >> + gpio->label = dev_name(dev);
> >> + gpio->parent = dev;
> >> + gpio->owner = THIS_MODULE;
> >> + gpio->of_node = dev->of_node;
> >> + gpio->ngpio = 2;
> >> + gpio->base = -1;
> >> + gpio->set = max9286_gpio_set;
> >> + gpio->get = max9286_gpio_get;
> >> + gpio->can_sleep = true;
> >> + gpio->names = names;
> >> +
> >> + /* GPIO values default to high */
> >> + priv->gpio_state = BIT(0) | BIT(1);
> >> +
> >> + ret = devm_gpiochip_add_data(dev, gpio, priv);
> >> + if (ret)
> >> + dev_err(dev, "Unable to create gpio_chip\n");
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int max9286_init(struct device *dev)
> >> +{
> >> + struct max9286_priv *priv;
> >> + struct i2c_client *client;
> >> + int ret;
> >> +
> >> + client = to_i2c_client(dev);
> >> + priv = i2c_get_clientdata(client);
> >> +
> >> + /* Enable the bus power. */
> >> + ret = regulator_enable(priv->regulator);
> >> + if (ret < 0) {
> >> + dev_err(&client->dev, "Unable to turn PoC on\n");
> >> + return ret;
> >> + }
> >> +
> >> + ret = max9286_setup(priv);
> >> + if (ret) {
> >> + dev_err(dev, "Unable to setup max9286\n");
> >> + goto err_regulator;
> >> + }
> >> +
> >> + /*
> >> + * Register all V4L2 interactions for the MAX9286 and notifiers for
> >> + * any subdevices connected.
> >> + */
> >> + ret = max9286_v4l2_register(priv);
> >> + if (ret) {
> >> + dev_err(dev, "Failed to register with V4L2\n");
> >> + goto err_regulator;
> >> + }
> >> +
> >> + ret = max9286_i2c_mux_init(priv);
> >> + if (ret) {
> >> + dev_err(dev, "Unable to initialize I2C multiplexer\n");
> >> + goto err_v4l2_register;
> >> + }
> >> +
> >> + /* Leave the mux channels disabled until they are selected. */
> >> + max9286_i2c_mux_close(priv);
> >> +
> >> + return 0;
> >> +
> >> +err_v4l2_register:
> >> + max9286_v4l2_unregister(priv);
> >> +err_regulator:
> >> + regulator_disable(priv->regulator);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void max9286_cleanup_dt(struct max9286_priv *priv)
> >> +{
> >> + struct max9286_source *source;
> >> +
> >> + for_each_source(priv, source) {
> >> + fwnode_handle_put(source->fwnode);
> >> + source->fwnode = NULL;
> >> + }
> >> +}
> >> +
> >> +static int max9286_parse_dt(struct max9286_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct device_node *i2c_mux;
> >> + struct device_node *node = NULL;
> >> + unsigned int i2c_mux_mask = 0;
> >> +
> >> + of_node_get(dev->of_node);
> >> + i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
> >> + if (!i2c_mux) {
> >> + dev_err(dev, "Failed to find i2c-mux node\n");
> >> + of_node_put(dev->of_node);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + /* Identify which i2c-mux channels are enabled */
> >> + for_each_child_of_node(i2c_mux, node) {
> >> + u32 id = 0;
> >> +
> >> + of_property_read_u32(node, "reg", &id);
> >> + if (id >= MAX9286_NUM_GMSL)
> >> + continue;
> >> +
> >> + if (!of_device_is_available(node)) {
> >> + dev_dbg(dev, "Skipping disabled I2C bus port %u\n", id);
> >> + continue;
> >> + }
> >> +
> >> + i2c_mux_mask |= BIT(id);
> >> + }
> >> + of_node_put(node);
> >> + of_node_put(i2c_mux);
> >> +
> >> + /* Parse the endpoints */
> >> + for_each_endpoint_of_node(dev->of_node, node) {
> >> + struct max9286_source *source;
> >> + struct of_endpoint ep;
> >> +
> >> + of_graph_parse_endpoint(node, &ep);
> >> + dev_dbg(dev, "Endpoint %pOF on port %d",
> >> + ep.local_node, ep.port);
> >> +
> >> + if (ep.port > MAX9286_NUM_GMSL) {
> >> + dev_err(dev, "Invalid endpoint %s on port %d",
> >> + of_node_full_name(ep.local_node), ep.port);
> >> + continue;
> >> + }
> >> +
> >> + /* For the source endpoint just parse the bus configuration. */
> >> + if (ep.port == MAX9286_SRC_PAD) {
> >> + struct v4l2_fwnode_endpoint vep = {
> >> + .bus_type = V4L2_MBUS_CSI2_DPHY
> >> + };
> >> + int ret;
> >> +
> >> + ret = v4l2_fwnode_endpoint_parse(
> >> + of_fwnode_handle(node), &vep);
> >> + if (ret) {
> >> + of_node_put(node);
> >> + of_node_put(dev->of_node);
> >> + return ret;
> >> + }
> >> +
> >> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
> >> + dev_err(dev,
> >> + "Media bus %u type not supported\n",
> >> + vep.bus_type);
> >> + v4l2_fwnode_endpoint_free(&vep);
> >> + of_node_put(node);
> >> + of_node_put(dev->of_node);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + priv->csi2_data_lanes =
> >> + vep.bus.mipi_csi2.num_data_lanes;
> >> + v4l2_fwnode_endpoint_free(&vep);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + /* Skip if the corresponding GMSL link is unavailable. */
> >> + if (!(i2c_mux_mask & BIT(ep.port)))
> >> + continue;
> >> +
> >> + if (priv->sources[ep.port].fwnode) {
> >> + dev_err(dev,
> >> + "Multiple port endpoints are not supported: %d",
> >> + ep.port);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + source = &priv->sources[ep.port];
> >> + source->fwnode = fwnode_graph_get_remote_endpoint(
> >> + of_fwnode_handle(node));
> >> + if (!source->fwnode) {
> >> + dev_err(dev,
> >> + "Endpoint %pOF has no remote endpoint connection\n",
> >> + ep.local_node);
> >> +
> >> + continue;
> >> + }
> >> +
> >> + priv->source_mask |= BIT(ep.port);
> >> + priv->nsources++;
> >> + }
> >> + of_node_put(node);
> >> + of_node_put(dev->of_node);
> >> +
> >> + priv->route_mask = priv->source_mask;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int max9286_probe(struct i2c_client *client)
> >> +{
> >> + struct max9286_priv *priv;
> >> + int ret;
> >> +
> >> + priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL);
> >> + if (!priv)
> >> + return -ENOMEM;
> >> +
> >> + mutex_init(&priv->mutex);
> >> +
> >> + priv->client = client;
> >> + i2c_set_clientdata(client, priv);
> >> +
> >> + priv->gpiod_pwdn = devm_gpiod_get_optional(&client->dev, "enable",
> >> + GPIOD_OUT_HIGH);
> >> + if (IS_ERR(priv->gpiod_pwdn))
> >> + return PTR_ERR(priv->gpiod_pwdn);
> >> +
> >> + gpiod_set_consumer_name(priv->gpiod_pwdn, "max9286-pwdn");
> >> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 1);
> >> +
> >> + /* Wait at least 4ms before the I2C lines latch to the address */
> >> + if (priv->gpiod_pwdn)
> >> + usleep_range(4000, 5000);
> >> +
> >> + /*
> >> + * The MAX9286 starts by default with all ports enabled, we disable all
> >> + * ports early to ensure that all channels are disabled if we error out
> >> + * and keep the bus consistent.
> >> + */
> >> + max9286_i2c_mux_close(priv);
> >> +
> >> + /*
> >> + * The MAX9286 initialises with auto-acknowledge enabled by default.
> >> + * This can be invasive to other transactions on the same bus, so
> >> + * disable it early. It will be enabled only as and when needed.
> >> + */
> >> + max9286_configure_i2c(priv, false);
> >> +
> >> + ret = max9286_register_gpio(priv);
> >> + if (ret)
> >> + goto err_powerdown;
> >> +
> >> + priv->regulator = devm_regulator_get(&client->dev, "poc");
> >> + if (IS_ERR(priv->regulator)) {
> >> + if (PTR_ERR(priv->regulator) != -EPROBE_DEFER)
> >> + dev_err(&client->dev,
> >> + "Unable to get PoC regulator (%ld)\n",
> >> + PTR_ERR(priv->regulator));
> >> + ret = PTR_ERR(priv->regulator);
> >> + goto err_powerdown;
> >> + }
> >> +
> >> + ret = max9286_parse_dt(priv);
> >> + if (ret)
> >> + goto err_powerdown;
> >> +
> >> + ret = max9286_init(&client->dev);
> >> + if (ret < 0)
> >> + goto err_cleanup_dt;
> >> +
> >> + return 0;
> >> +
> >> +err_cleanup_dt:
> >> + max9286_cleanup_dt(priv);
> >> +err_powerdown:
> >> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int max9286_remove(struct i2c_client *client)
> >> +{
> >> + struct max9286_priv *priv = i2c_get_clientdata(client);
> >> +
> >> + i2c_mux_del_adapters(priv->mux);
> >> +
> >> + max9286_v4l2_unregister(priv);
> >> +
> >> + regulator_disable(priv->regulator);
> >> +
> >> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
> >> +
> >> + max9286_cleanup_dt(priv);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct of_device_id max9286_dt_ids[] = {
> >> + { .compatible = "maxim,max9286" },
> >> + {},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, max9286_dt_ids);
> >> +
> >> +static struct i2c_driver max9286_i2c_driver = {
> >> + .driver = {
> >> + .name = "max9286",
> >> + .of_match_table = of_match_ptr(max9286_dt_ids),
> >> + },
> >> + .probe_new = max9286_probe,
> >> + .remove = max9286_remove,
> >> +};
> >> +
> >> +module_i2c_driver(max9286_i2c_driver);
> >> +
> >> +MODULE_DESCRIPTION("Maxim MAX9286 GMSL Deserializer Driver");
> >> +MODULE_AUTHOR("Jacopo Mondi, Kieran Bingham, Laurent Pinchart, Niklas Söderlund, Vladimir Barinov");
> >> +MODULE_LICENSE("GPL");
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply
* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Kieran Bingham @ 2020-05-14 10:02 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Rob Herring, Jacopo Mondi, Laurent Pinchart,
Niklas Söderlund
In-Reply-To: <20200512181706.GA21014@Mani-XPS-13-9360>
Hi Mani,
On 12/05/2020 19:17, Manivannan Sadhasivam wrote:
> On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:
>> The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
>> CSI-2 output. The device supports multicamera streaming applications,
>> and features the ability to synchronise the attached cameras.
>>
>> CSI-2 output can be configured with 1 to 4 lanes, and a control channel
>> is supported over I2C, which implements an I2C mux to facilitate
>> communications with connected cameras across the reverse control
>> channel.
>>
>> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>
>> --
>> v2:
>> - Fix MAINTAINERS entry
>>
>> This posting is released with the following modifications to work
>> without Sakari's VC developments:
>> - max9286_g_mbus_config() re-instated
>> - max9286_get_frame_desc() is not bus/csi aware
>> - max9286_{get,set}_routing() removed
>>
>> v3:
>> - Initialise notifier with v4l2_async_notifier_init
>> - Update for new mbus csi2 format V4L2_MBUS_CSI2_DPHY
>>
>> v4: - Re-introduce required code to function with the VC series.
>>
>> - Implement max9286_get_routing, max9286_set_routing
>> - Remove max9286_g_mbus_config
>>
>> v5: (internal release)
>> - Fix printk formatting for hex value
>> - max9286->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE (add |)
>> - MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER -> MEDIA_ENT_F_VID_IF_BRIDGE
>> - Remove 'device is bound' workaround
>>
>> v6:
>> - v4l2_subdev_krouting instead of v4l2_subdev_routing separated
>> to allow integration without the VC/V4L2-Mux series.
>> - convert sd_to_max9286 to inline function
>> - rename max9286_device to max9286_priv
>> - Cleanup the v4l2_async_notifier
>> - Extend MODULE_AUTHOR
>> - Replace of_graph_get_endpoint_by_regs with fwnode_graph_get_endpoint_by_id
>> - Pass default bus type when parsing fwnode endpoint (Manivannan Sadhasivam)
>> - Use new YAML file reference in MAINTAINERS
>> - Parse new i2c-mux node in max9286_get_i2c_by_id
>> (This could/should be refactored to parse these separately first)
>> - Spelling and calculation fixes in the FSYNC_LOCKED check comments
>> - Identify each enabled i2c-mux channel in a single pass
>> - max9286: Improve mux-state readbility [v2]
>> - Fix frame sync lock durations
>> - Add comment to describe /why/ we must open the mux in s_stream
>> - use -EXDEV as return code for failed link synchronisation.
>> - Fix reference counting of the dt nodeS
>> - Convert to probe_new for I2C
>> - Remove redundant max9286_i2c_mux_state
>> - Provide optional enable-gpio (max9286-pwdn)
>>
>> v7:
>> [Kieran]
>> - Ensure powerdown lines are optional
>> - Add a 4ms power-up delay
>> - Add max9286_check_config_link() to core
>> - Add GPIO chip controller for GPIO0OUT and GPIO1OUT
>> - Fix GPIO registration
>> - max9286: Split out async registration
>> (fixes regulator -EPROBE_DEFERs failures)
>> - Collect all V4L2 registrations
>> - balance v4l2_async refcnting
>> - Rename max9286_v4l2_async_ => max9286_v4l2_notifier_
>>
>> [Jacopo]
>> - Remove redundanct MAXIM_I2C_SPEED macros
>> - Move notifiers operations
>> - Add delay after reverse channel reconfiguration
>> - Move link setup to completion
>> - Fix up max9286_check_config_link() implementation
>> - Remove redundant dual configuration of reverse channel
>>
>> v8:
>>
>> [Kieran]
>> - Update the bound_sources mask on unbind
>> - Convert probe kzalloc usage to devm_ variant
>> - Fix up cleanup path from GPIO PowerDown registration
>> - cleanup GPIO device registration fail path
>> - Convert to use devm_regulator_get()
>> - Fit max9286_parse_dt print on one line
>> - Move multi-device workarounds out of upstream driver
>> - Remove I2C mod-table
>> - Lock format changes
>> - Describe pad index usage
>> - Remove poc_enabled workaround
>> - Rename the max9286_gpio to be more explicit on it's actions.
>> - Move max9286_init_format call
>> - Rework probe sequence and simplify error paths.
>> - Simplify i2c comments
>> - Implement Pixelrate control
>> - Disable overlap window
>>
>> [Jacopo]
>> - Adapt Kconfig to latest upstream changes
>> - Put of node on error
>> - Calculate pixel rate
>> - Simplify overlap window disablement
>>
>> v9:
>>
>> [Kieran]
>> - Kconfig: Depend on OF
>> - Re-sort addition to Makefile
>> ---
>> MAINTAINERS | 10 +
>> drivers/media/i2c/Kconfig | 13 +
>> drivers/media/i2c/Makefile | 1 +
>> drivers/media/i2c/max9286.c | 1332 +++++++++++++++++++++++++++++++++++
>> 4 files changed, 1356 insertions(+)
>> create mode 100644 drivers/media/i2c/max9286.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a7bb6e22d5da..99e3bf7760fd 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -10274,6 +10274,16 @@ F: Documentation/hwmon/max6697.rst
>> F: drivers/hwmon/max6697.c
>> F: include/linux/platform_data/max6697.h
>>
>> +MAX9286 QUAD GMSL DESERIALIZER DRIVER
>> +M: Jacopo Mondi <jacopo+renesas@jmondi.org>
>> +M: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>> +M: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> +M: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>> +L: linux-media@vger.kernel.org
>> +S: Maintained
>> +F: Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
>> +F: drivers/media/i2c/max9286.c
>> +
>> MAX9860 MONO AUDIO VOICE CODEC DRIVER
>> M: Peter Rosin <peda@axentia.se>
>> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 3abc80373ec0..2e390f41f6da 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -464,6 +464,19 @@ config VIDEO_VPX3220
>> To compile this driver as a module, choose M here: the
>> module will be called vpx3220.
>>
>> +config VIDEO_MAX9286
>> + tristate "Maxim MAX9286 GMSL deserializer support"
>> + depends on I2C && I2C_MUX
>> + depends on OF
>> + select V4L2_FWNODE
>> + select VIDEO_V4L2_SUBDEV_API
>> + select MEDIA_CONTROLLER
>> + help
>> + This driver supports the Maxim MAX9286 GMSL deserializer.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called max9286.
>> +
>> comment "Video and audio decoders"
>>
>> config VIDEO_SAA717X
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 77bf7d0b691f..f0b001ee4b05 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -117,6 +117,7 @@ obj-$(CONFIG_VIDEO_IMX274) += imx274.o
>> obj-$(CONFIG_VIDEO_IMX290) += imx290.o
>> obj-$(CONFIG_VIDEO_IMX319) += imx319.o
>> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
>> +obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
>> obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
>>
>> obj-$(CONFIG_SDR_MAX2175) += max2175.o
>> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
>> new file mode 100644
>> index 000000000000..481d65f2b51d
>> --- /dev/null
>> +++ b/drivers/media/i2c/max9286.c
>> @@ -0,0 +1,1332 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Maxim MAX9286 GMSL Deserializer Driver
>> + *
>> + * Copyright (C) 2017-2019 Jacopo Mondi
>> + * Copyright (C) 2017-2019 Kieran Bingham
>> + * Copyright (C) 2017-2019 Laurent Pinchart
>> + * Copyright (C) 2017-2019 Niklas Söderlund
>> + * Copyright (C) 2016 Renesas Electronics Corporation
>> + * Copyright (C) 2015 Cogent Embedded, Inc.
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/device.h>
>> +#include <linux/fwnode.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/i2c.h>
>> +#include <linux/i2c-mux.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_graph.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +
>> +#include <media/v4l2-async.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-device.h>
>> +#include <media/v4l2-fwnode.h>
>> +#include <media/v4l2-subdev.h>
>> +
>> +/* Register 0x00 */
>> +#define MAX9286_MSTLINKSEL_AUTO (7 << 5)
>> +#define MAX9286_MSTLINKSEL(n) ((n) << 5)
>> +#define MAX9286_EN_VS_GEN BIT(4)
>> +#define MAX9286_LINKEN(n) (1 << (n))
>> +/* Register 0x01 */
>> +#define MAX9286_FSYNCMODE_ECU (3 << 6)
>> +#define MAX9286_FSYNCMODE_EXT (2 << 6)
>> +#define MAX9286_FSYNCMODE_INT_OUT (1 << 6)
>> +#define MAX9286_FSYNCMODE_INT_HIZ (0 << 6)
>> +#define MAX9286_GPIEN BIT(5)
>> +#define MAX9286_ENLMO_RSTFSYNC BIT(2)
>> +#define MAX9286_FSYNCMETH_AUTO (2 << 0)
>> +#define MAX9286_FSYNCMETH_SEMI_AUTO (1 << 0)
>> +#define MAX9286_FSYNCMETH_MANUAL (0 << 0)
>> +#define MAX9286_REG_FSYNC_PERIOD_L 0x06
>> +#define MAX9286_REG_FSYNC_PERIOD_M 0x07
>> +#define MAX9286_REG_FSYNC_PERIOD_H 0x08
>> +/* Register 0x0a */
>> +#define MAX9286_FWDCCEN(n) (1 << ((n) + 4))
>> +#define MAX9286_REVCCEN(n) (1 << (n))
>> +/* Register 0x0c */
>> +#define MAX9286_HVEN BIT(7)
>> +#define MAX9286_EDC_6BIT_HAMMING (2 << 5)
>> +#define MAX9286_EDC_6BIT_CRC (1 << 5)
>> +#define MAX9286_EDC_1BIT_PARITY (0 << 5)
>> +#define MAX9286_DESEL BIT(4)
>> +#define MAX9286_INVVS BIT(3)
>> +#define MAX9286_INVHS BIT(2)
>> +#define MAX9286_HVSRC_D0 (2 << 0)
>> +#define MAX9286_HVSRC_D14 (1 << 0)
>> +#define MAX9286_HVSRC_D18 (0 << 0)
>> +/* Register 0x0f */
>> +#define MAX9286_0X0F_RESERVED BIT(3)
>> +/* Register 0x12 */
>> +#define MAX9286_CSILANECNT(n) (((n) - 1) << 6)
>> +#define MAX9286_CSIDBL BIT(5)
>> +#define MAX9286_DBL BIT(4)
>> +#define MAX9286_DATATYPE_USER_8BIT (11 << 0)
>> +#define MAX9286_DATATYPE_USER_YUV_12BIT (10 << 0)
>> +#define MAX9286_DATATYPE_USER_24BIT (9 << 0)
>> +#define MAX9286_DATATYPE_RAW14 (8 << 0)
>> +#define MAX9286_DATATYPE_RAW11 (7 << 0)
>> +#define MAX9286_DATATYPE_RAW10 (6 << 0)
>> +#define MAX9286_DATATYPE_RAW8 (5 << 0)
>> +#define MAX9286_DATATYPE_YUV422_10BIT (4 << 0)
>> +#define MAX9286_DATATYPE_YUV422_8BIT (3 << 0)
>> +#define MAX9286_DATATYPE_RGB555 (2 << 0)
>> +#define MAX9286_DATATYPE_RGB565 (1 << 0)
>> +#define MAX9286_DATATYPE_RGB888 (0 << 0)
>> +/* Register 0x15 */
>> +#define MAX9286_VC(n) ((n) << 5)
>> +#define MAX9286_VCTYPE BIT(4)
>> +#define MAX9286_CSIOUTEN BIT(3)
>> +#define MAX9286_0X15_RESV (3 << 0)
>> +/* Register 0x1b */
>> +#define MAX9286_SWITCHIN(n) (1 << ((n) + 4))
>> +#define MAX9286_ENEQ(n) (1 << (n))
>> +/* Register 0x27 */
>> +#define MAX9286_LOCKED BIT(7)
>> +/* Register 0x31 */
>> +#define MAX9286_FSYNC_LOCKED BIT(6)
>> +/* Register 0x34 */
>> +#define MAX9286_I2CLOCACK BIT(7)
>> +#define MAX9286_I2CSLVSH_1046NS_469NS (3 << 5)
>> +#define MAX9286_I2CSLVSH_938NS_352NS (2 << 5)
>> +#define MAX9286_I2CSLVSH_469NS_234NS (1 << 5)
>> +#define MAX9286_I2CSLVSH_352NS_117NS (0 << 5)
>> +#define MAX9286_I2CMSTBT_837KBPS (7 << 2)
>> +#define MAX9286_I2CMSTBT_533KBPS (6 << 2)
>> +#define MAX9286_I2CMSTBT_339KBPS (5 << 2)
>> +#define MAX9286_I2CMSTBT_173KBPS (4 << 2)
>> +#define MAX9286_I2CMSTBT_105KBPS (3 << 2)
>> +#define MAX9286_I2CMSTBT_84KBPS (2 << 2)
>> +#define MAX9286_I2CMSTBT_28KBPS (1 << 2)
>> +#define MAX9286_I2CMSTBT_8KBPS (0 << 2)
>> +#define MAX9286_I2CSLVTO_NONE (3 << 0)
>> +#define MAX9286_I2CSLVTO_1024US (2 << 0)
>> +#define MAX9286_I2CSLVTO_256US (1 << 0)
>> +#define MAX9286_I2CSLVTO_64US (0 << 0)
>> +/* Register 0x3b */
>> +#define MAX9286_REV_TRF(n) ((n) << 4)
>> +#define MAX9286_REV_AMP(n) ((((n) - 30) / 10) << 1) /* in mV */
>> +#define MAX9286_REV_AMP_X BIT(0)
>> +/* Register 0x3f */
>> +#define MAX9286_EN_REV_CFG BIT(6)
>> +#define MAX9286_REV_FLEN(n) ((n) - 20)
>> +/* Register 0x49 */
>> +#define MAX9286_VIDEO_DETECT_MASK 0x0f
>> +/* Register 0x69 */
>> +#define MAX9286_LFLTBMONMASKED BIT(7)
>> +#define MAX9286_LOCKMONMASKED BIT(6)
>> +#define MAX9286_AUTOCOMBACKEN BIT(5)
>> +#define MAX9286_AUTOMASKEN BIT(4)
>> +#define MAX9286_MASKLINK(n) ((n) << 0)
>> +
>> +/*
>> + * The sink and source pads are created to match the OF graph port numbers so
>> + * that their indexes can be used interchangeably.
>> + */
>> +#define MAX9286_NUM_GMSL 4
>> +#define MAX9286_N_SINKS 4
>> +#define MAX9286_N_PADS 5
>> +#define MAX9286_SRC_PAD 4
>> +
>> +struct max9286_source {
>> + struct v4l2_async_subdev asd;
>> + struct v4l2_subdev *sd;
>> + struct fwnode_handle *fwnode;
>> +};
>> +
>> +#define asd_to_max9286_source(_asd) \
>> + container_of(_asd, struct max9286_source, asd)
>> +
>> +struct max9286_priv {
>> + struct i2c_client *client;
>> + struct gpio_desc *gpiod_pwdn;
>> + struct v4l2_subdev sd;
>> + struct media_pad pads[MAX9286_N_PADS];
>> + struct regulator *regulator;
>> +
>> + struct gpio_chip gpio;
>> + u8 gpio_state;
>> +
>> + struct i2c_mux_core *mux;
>> + unsigned int mux_channel;
>> + bool mux_open;
>> +
>> + struct v4l2_ctrl_handler ctrls;
>> + struct v4l2_ctrl *pixelrate;
>> +
>> + struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
>> +
>> + /* Protects controls and fmt structures */
>> + struct mutex mutex;
>> +
>> + unsigned int nsources;
>> + unsigned int source_mask;
>> + unsigned int route_mask;
>> + unsigned int bound_sources;
>> + unsigned int csi2_data_lanes;
>> + struct max9286_source sources[MAX9286_NUM_GMSL];
>> + struct v4l2_async_notifier notifier;
>> +};
>> +
>
> [...]
>
>> +static int max9286_register_gpio(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct gpio_chip *gpio = &priv->gpio;
>> + int ret;
>> +
>> + static const char * const names[] = {
>> + "GPIO0OUT",
>> + "GPIO1OUT",
>> + };
>> +
>> + /* Configure the GPIO */
>> + gpio->label = dev_name(dev);
>
> So if you have more than one MAX9286 in a system, all gpiochips will appear
> with the same name. I'd recommend to append the index to distinguish properly.
Ah yes, that's a good point, and I think I've even seen that.
I'll fix it now.
>
>> + gpio->parent = dev;
>> + gpio->owner = THIS_MODULE;
>> + gpio->of_node = dev->of_node;
>> + gpio->ngpio = 2;
>> + gpio->base = -1;
>> + gpio->set = max9286_gpio_set;
>> + gpio->get = max9286_gpio_get;
>> + gpio->can_sleep = true;
>> + gpio->names = names;
>> +
>> + /* GPIO values default to high */
>> + priv->gpio_state = BIT(0) | BIT(1);
>> +
>> + ret = devm_gpiochip_add_data(dev, gpio, priv);
>> + if (ret)
>> + dev_err(dev, "Unable to create gpio_chip\n");
>> +
>> + return ret;
>> +}
>> +
>
> [...]
>
>> +static int max9286_parse_dt(struct max9286_priv *priv)
>> +{
>> + struct device *dev = &priv->client->dev;
>> + struct device_node *i2c_mux;
>> + struct device_node *node = NULL;
>> + unsigned int i2c_mux_mask = 0;
>> +
>> + of_node_get(dev->of_node);
>
> Why this is needed?
Hrm .. I recall adding it to solve dt reference balancing.
I wish I'd added a comment at the time ... as I can't recall the details
now.
>> + i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
>> + if (!i2c_mux) {
>> + dev_err(dev, "Failed to find i2c-mux node\n");
>> + of_node_put(dev->of_node);
>> + return -EINVAL;
>> + }
>> +
>> + /* Identify which i2c-mux channels are enabled */
>> + for_each_child_of_node(i2c_mux, node) {
>> + u32 id = 0;
>> +
>> + of_property_read_u32(node, "reg", &id);
>> + if (id >= MAX9286_NUM_GMSL)
>> + continue;
>> +
>> + if (!of_device_is_available(node)) {
>> + dev_dbg(dev, "Skipping disabled I2C bus port %u\n", id);
>> + continue;
>> + }
>> +
>> + i2c_mux_mask |= BIT(id);
>> + }
>> + of_node_put(node);
>> + of_node_put(i2c_mux);
>> +
>> + /* Parse the endpoints */
>> + for_each_endpoint_of_node(dev->of_node, node) {
>> + struct max9286_source *source;
>> + struct of_endpoint ep;
>> +
>> + of_graph_parse_endpoint(node, &ep);
>> + dev_dbg(dev, "Endpoint %pOF on port %d",
>> + ep.local_node, ep.port);
>> +
>> + if (ep.port > MAX9286_NUM_GMSL) {
>> + dev_err(dev, "Invalid endpoint %s on port %d",
>> + of_node_full_name(ep.local_node), ep.port);
>> + continue;
>> + }
>> +
>> + /* For the source endpoint just parse the bus configuration. */
>> + if (ep.port == MAX9286_SRC_PAD) {
>> + struct v4l2_fwnode_endpoint vep = {
>> + .bus_type = V4L2_MBUS_CSI2_DPHY
>> + };
>> + int ret;
>> +
>> + ret = v4l2_fwnode_endpoint_parse(
>> + of_fwnode_handle(node), &vep);
>> + if (ret) {
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> + return ret;
>> + }
>> +
>> + if (vep.bus_type != V4L2_MBUS_CSI2_DPHY) {
>> + dev_err(dev,
>> + "Media bus %u type not supported\n",
>> + vep.bus_type);
>> + v4l2_fwnode_endpoint_free(&vep);
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> + return -EINVAL;
>> + }
>> +
>> + priv->csi2_data_lanes =
>> + vep.bus.mipi_csi2.num_data_lanes;
>> + v4l2_fwnode_endpoint_free(&vep);
>> +
>> + continue;
>> + }
>> +
>> + /* Skip if the corresponding GMSL link is unavailable. */
>> + if (!(i2c_mux_mask & BIT(ep.port)))
>> + continue;
>> +
>> + if (priv->sources[ep.port].fwnode) {
>> + dev_err(dev,
>> + "Multiple port endpoints are not supported: %d",
>> + ep.port);
>> +
>> + continue;
>> + }
>> +
>> + source = &priv->sources[ep.port];
>> + source->fwnode = fwnode_graph_get_remote_endpoint(
>> + of_fwnode_handle(node));
>> + if (!source->fwnode) {
>> + dev_err(dev,
>> + "Endpoint %pOF has no remote endpoint connection\n",
>> + ep.local_node);
>> +
>> + continue;
>> + }
>> +
>> + priv->source_mask |= BIT(ep.port);
>> + priv->nsources++;
>> + }
>> + of_node_put(node);
>> + of_node_put(dev->of_node);
>> +
>> + priv->route_mask = priv->source_mask;
>> +
>> + return 0;
>> +}
>> +
>
> [...]
>
>> +static int max9286_remove(struct i2c_client *client)
>> +{
>> + struct max9286_priv *priv = i2c_get_clientdata(client);
>> +
>> + i2c_mux_del_adapters(priv->mux);
>> +
>> + max9286_v4l2_unregister(priv);
>> +
>> + regulator_disable(priv->regulator);
>> +
>> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
>
> Usual power down sequence is to pull the power down gpio low and then turn off
> the regulators. This helps in clearing up the internal state machine properly.
Do you mean usual, among drivers using regulators? or usual for the max9286?
I have a platform which controls the regulators for the cameras
(priv->regulator) through one of the GPIOs provided by the MAX9286.
If we powerdown the max9286 first, then we will not be able to change
the GPIO value on the gpio controlled regulator.
(Currently it doesn't expose as a gpio regulator, but just a gpio-hog -
but thats a different matter)
--
Kieran
>
> Thanks,
> Mani
>
>> +
>> + max9286_cleanup_dt(priv);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id max9286_dt_ids[] = {
>> + { .compatible = "maxim,max9286" },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, max9286_dt_ids);
>> +
>> +static struct i2c_driver max9286_i2c_driver = {
>> + .driver = {
>> + .name = "max9286",
>> + .of_match_table = of_match_ptr(max9286_dt_ids),
>> + },
>> + .probe_new = max9286_probe,
>> + .remove = max9286_remove,
>> +};
>> +
>> +module_i2c_driver(max9286_i2c_driver);
>> +
>> +MODULE_DESCRIPTION("Maxim MAX9286 GMSL Deserializer Driver");
>> +MODULE_AUTHOR("Jacopo Mondi, Kieran Bingham, Laurent Pinchart, Niklas Söderlund, Vladimir Barinov");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.25.1
>>
^ permalink raw reply
* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Kieran Bingham @ 2020-05-14 10:05 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Rob Herring, Jacopo Mondi, Laurent Pinchart,
Niklas Söderlund
In-Reply-To: <20200514095948.GE2877@Mani-XPS-13-9360>
Hi Mani,
On 14/05/2020 10:59, Manivannan Sadhasivam wrote:
> On Thu, May 14, 2020 at 10:53:00AM +0100, Kieran Bingham wrote:
>> Hi Mani,
>>>> +static int max9286_set_fmt(struct v4l2_subdev *sd,
>>>> + struct v4l2_subdev_pad_config *cfg,
>>>> + struct v4l2_subdev_format *format)
>>>> +{
>>>> + struct max9286_priv *priv = sd_to_max9286(sd);
>>>> + struct v4l2_mbus_framefmt *cfg_fmt;
>>>> +
>>>> + if (format->pad >= MAX9286_SRC_PAD)
>>>> + return -EINVAL;
>>>> +
>>>> + /* Refuse non YUV422 formats as we hardcode DT to 8 bit YUV422 */
>>>> + switch (format->format.code) {
>>>> + case MEDIA_BUS_FMT_UYVY8_2X8:
>>>> + case MEDIA_BUS_FMT_VYUY8_2X8:
>>>> + case MEDIA_BUS_FMT_YUYV8_2X8:
>>>> + case MEDIA_BUS_FMT_YVYU8_2X8:
>>>> + break;
>>>> + default:
>>>> + format->format.code = MEDIA_BUS_FMT_YUYV8_2X8;
>>>
>>> Is there any reason for not setting default format to MEDIA_BUS_FMT_UYVY8_2X8?
>>>
>>
>>
>> No good reason no, and I see that in max9286_enum_mbus_code(), we
>> currently code to MEDIA_BUS_FMT_YUYV8_2X8, and that's the value that we
>> init to, so that would be a better default indeed.
>>
>
> max9286_enum_mbus_code() returns MEDIA_BUS_FMT_UYVY8_2X8, no?
Argh, sorry - yet anther typo or copy/paste error.
Indeed, I have changed this locally to ...
<double checks> MEDIA_BUS_FMT_UYVY8_2X8 </double checks>
as suggested. ;-)
>
> Thanks,
> Mani
^ permalink raw reply
* [PATCH v4] dt-bindings: display: anx7814.txt: convert to yaml
From: Ricardo Cañuelo @ 2020-05-14 10:12 UTC (permalink / raw)
To: devicetree; +Cc: kernel, dri-devel, robh+dt, enric.balletbo
This converts the Analogix ANX7814 bridge DT binding to yaml. Port
definitions and descriptions were expanded, apart from that it's a
direct translation from the original binding.
Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
Changes in v4:
- Make interrupts and gpios optional (Enric Balletbo)
- Make hpd-gpios deprecated (Rob Herring)
- Remove maxItems from dvdd10-supply
Changes in v3 (suggested by Sam Ravnborg):
- Rename example node i2c0 to i2c.
Changes in v2 (suggested by Enric Balletbo):
- File name change: use full compatible string.
- Binding description removed.
- #address-cells and #size-cells properties removed from ports node.
- Example node renamed: anx7814 -> bridge.
Tested with:
make dt_binding_check ARCH=arm64 DT_SCHEMA_FILES=<.../analogix,anx7814.yaml>
make dtbs_check ARCH=arm64 DT_SCHEMA_FILES=<.../analogix,anx7814.yaml>
.../display/bridge/analogix,anx7814.yaml | 119 ++++++++++++++++++
.../bindings/display/bridge/anx7814.txt | 42 -------
2 files changed, 119 insertions(+), 42 deletions(-)
create mode 100644 Documentation/devicetree/bindings/display/bridge/analogix,anx7814.yaml
delete mode 100644 Documentation/devicetree/bindings/display/bridge/anx7814.txt
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7814.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7814.yaml
new file mode 100644
index 000000000000..3ba477aefdd7
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7814.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/analogix,anx7814.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analogix ANX7814 SlimPort (Full-HD Transmitter)
+
+maintainers:
+ - Enric Balletbo i Serra <enric.balletbo@collabora.com>
+
+properties:
+ compatible:
+ enum:
+ - analogix,anx7808
+ - analogix,anx7812
+ - analogix,anx7814
+ - analogix,anx7818
+
+ reg:
+ maxItems: 1
+ description: I2C address of the device.
+
+ interrupts:
+ maxItems: 1
+ description: Should contain the INTP interrupt.
+
+ hpd-gpios:
+ deprecated: true
+ maxItems: 1
+ description: Which GPIO to use for hpd.
+
+ pd-gpios:
+ maxItems: 1
+ description: Which GPIO to use for power down.
+
+ reset-gpios:
+ maxItems: 1
+ description: Which GPIO to use for reset.
+
+ dvdd10-supply:
+ description: Regulator for 1.0V digital core power.
+
+ ports:
+ type: object
+ description:
+ A node containing input and output port nodes with endpoint
+ definitions as documented in
+ Documentation/devicetree/bindings/media/video-interfaces.txt
+ Documentation/devicetree/bindings/graph.txt
+
+ properties:
+ port@0:
+ type: object
+ description: Video port for HDMI input.
+
+ properties:
+ reg:
+ const: 0
+
+ port@1:
+ type: object
+ description:
+ Video port for SlimPort, DisplayPort, eDP or MyDP output.
+
+ properties:
+ reg:
+ const: 1
+
+ required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ anx7814: bridge@38 {
+ compatible = "analogix,anx7814";
+ reg = <0x38>;
+ interrupt-parent = <&gpio0>;
+ interrupts = <99 IRQ_TYPE_LEVEL_LOW>; /* INTP */
+ pd-gpios = <&pio 33 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&pio 98 GPIO_ACTIVE_HIGH>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ anx7814_in: endpoint {
+ remote-endpoint = <&hdmi0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ anx7814_out: endpoint {
+ remote-endpoint = <&edp_out>;
+ };
+ };
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/display/bridge/anx7814.txt b/Documentation/devicetree/bindings/display/bridge/anx7814.txt
deleted file mode 100644
index 17258747fff6..000000000000
--- a/Documentation/devicetree/bindings/display/bridge/anx7814.txt
+++ /dev/null
@@ -1,42 +0,0 @@
-Analogix ANX7814 SlimPort (Full-HD Transmitter)
------------------------------------------------
-
-The ANX7814 is an ultra-low power Full-HD (1080p60) SlimPort transmitter
-designed for portable devices.
-
-Required properties:
-
- - compatible : Must be one of:
- "analogix,anx7808"
- "analogix,anx7812"
- "analogix,anx7814"
- "analogix,anx7818"
- - reg : I2C address of the device
- - interrupts : Should contain the INTP interrupt
- - hpd-gpios : Which GPIO to use for hpd
- - pd-gpios : Which GPIO to use for power down
- - reset-gpios : Which GPIO to use for reset
-
-Optional properties:
-
- - dvdd10-supply : Regulator for 1.0V digital core power.
- - Video port for HDMI input, using the DT bindings defined in [1].
-
-[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
-
-Example:
-
- anx7814: anx7814@38 {
- compatible = "analogix,anx7814";
- reg = <0x38>;
- interrupt-parent = <&gpio0>;
- interrupts = <99 IRQ_TYPE_LEVEL_LOW>; /* INTP */
- hpd-gpios = <&pio 36 GPIO_ACTIVE_HIGH>;
- pd-gpios = <&pio 33 GPIO_ACTIVE_HIGH>;
- reset-gpios = <&pio 98 GPIO_ACTIVE_HIGH>;
- port {
- anx7814_in: endpoint {
- remote-endpoint = <&hdmi0_out>;
- };
- };
- };
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v9 2/4] media: i2c: Add MAX9286 driver
From: Manivannan Sadhasivam @ 2020-05-14 10:13 UTC (permalink / raw)
To: Kieran Bingham
Cc: linux-renesas-soc, linux-media, devicetree, linux-kernel,
Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
Jacopo Mondi, Niklas Söderlund, sakari.ailus, Hans Verkuil,
Hyun Kwon, Rob Herring, Jacopo Mondi, Laurent Pinchart,
Niklas Söderlund
In-Reply-To: <11aca587-9438-4fba-081c-b82631e96989@ideasonboard.com>
Hi Kieran,
On Thu, May 14, 2020 at 11:02:53AM +0100, Kieran Bingham wrote:
> Hi Mani,
>
> On 12/05/2020 19:17, Manivannan Sadhasivam wrote:
> > On Tue, May 12, 2020 at 04:51:03PM +0100, Kieran Bingham wrote:
> >> The MAX9286 is a 4-channel GMSL deserializer with coax or STP input and
> >> CSI-2 output. The device supports multicamera streaming applications,
> >> and features the ability to synchronise the attached cameras.
> >>
> >> CSI-2 output can be configured with 1 to 4 lanes, and a control channel
> >> is supported over I2C, which implements an I2C mux to facilitate
> >> communications with connected cameras across the reverse control
> >> channel.
> >>
> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >>
> >> --
> >> v2:
> >> - Fix MAINTAINERS entry
> >>
> >> This posting is released with the following modifications to work
> >> without Sakari's VC developments:
> >> - max9286_g_mbus_config() re-instated
> >> - max9286_get_frame_desc() is not bus/csi aware
> >> - max9286_{get,set}_routing() removed
> >>
> >> v3:
> >> - Initialise notifier with v4l2_async_notifier_init
> >> - Update for new mbus csi2 format V4L2_MBUS_CSI2_DPHY
> >>
> >> v4: - Re-introduce required code to function with the VC series.
> >>
> >> - Implement max9286_get_routing, max9286_set_routing
> >> - Remove max9286_g_mbus_config
> >>
> >> v5: (internal release)
> >> - Fix printk formatting for hex value
> >> - max9286->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE (add |)
> >> - MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER -> MEDIA_ENT_F_VID_IF_BRIDGE
> >> - Remove 'device is bound' workaround
> >>
> >> v6:
> >> - v4l2_subdev_krouting instead of v4l2_subdev_routing separated
> >> to allow integration without the VC/V4L2-Mux series.
> >> - convert sd_to_max9286 to inline function
> >> - rename max9286_device to max9286_priv
> >> - Cleanup the v4l2_async_notifier
> >> - Extend MODULE_AUTHOR
> >> - Replace of_graph_get_endpoint_by_regs with fwnode_graph_get_endpoint_by_id
> >> - Pass default bus type when parsing fwnode endpoint (Manivannan Sadhasivam)
> >> - Use new YAML file reference in MAINTAINERS
> >> - Parse new i2c-mux node in max9286_get_i2c_by_id
> >> (This could/should be refactored to parse these separately first)
> >> - Spelling and calculation fixes in the FSYNC_LOCKED check comments
> >> - Identify each enabled i2c-mux channel in a single pass
> >> - max9286: Improve mux-state readbility [v2]
> >> - Fix frame sync lock durations
> >> - Add comment to describe /why/ we must open the mux in s_stream
> >> - use -EXDEV as return code for failed link synchronisation.
> >> - Fix reference counting of the dt nodeS
> >> - Convert to probe_new for I2C
> >> - Remove redundant max9286_i2c_mux_state
> >> - Provide optional enable-gpio (max9286-pwdn)
> >>
> >> v7:
> >> [Kieran]
> >> - Ensure powerdown lines are optional
> >> - Add a 4ms power-up delay
> >> - Add max9286_check_config_link() to core
> >> - Add GPIO chip controller for GPIO0OUT and GPIO1OUT
> >> - Fix GPIO registration
> >> - max9286: Split out async registration
> >> (fixes regulator -EPROBE_DEFERs failures)
> >> - Collect all V4L2 registrations
> >> - balance v4l2_async refcnting
> >> - Rename max9286_v4l2_async_ => max9286_v4l2_notifier_
> >>
> >> [Jacopo]
> >> - Remove redundanct MAXIM_I2C_SPEED macros
> >> - Move notifiers operations
> >> - Add delay after reverse channel reconfiguration
> >> - Move link setup to completion
> >> - Fix up max9286_check_config_link() implementation
> >> - Remove redundant dual configuration of reverse channel
> >>
> >> v8:
> >>
> >> [Kieran]
> >> - Update the bound_sources mask on unbind
> >> - Convert probe kzalloc usage to devm_ variant
> >> - Fix up cleanup path from GPIO PowerDown registration
> >> - cleanup GPIO device registration fail path
> >> - Convert to use devm_regulator_get()
> >> - Fit max9286_parse_dt print on one line
> >> - Move multi-device workarounds out of upstream driver
> >> - Remove I2C mod-table
> >> - Lock format changes
> >> - Describe pad index usage
> >> - Remove poc_enabled workaround
> >> - Rename the max9286_gpio to be more explicit on it's actions.
> >> - Move max9286_init_format call
> >> - Rework probe sequence and simplify error paths.
> >> - Simplify i2c comments
> >> - Implement Pixelrate control
> >> - Disable overlap window
> >>
> >> [Jacopo]
> >> - Adapt Kconfig to latest upstream changes
> >> - Put of node on error
> >> - Calculate pixel rate
> >> - Simplify overlap window disablement
> >>
> >> v9:
> >>
> >> [Kieran]
> >> - Kconfig: Depend on OF
> >> - Re-sort addition to Makefile
> >> ---
> >> MAINTAINERS | 10 +
> >> drivers/media/i2c/Kconfig | 13 +
> >> drivers/media/i2c/Makefile | 1 +
> >> drivers/media/i2c/max9286.c | 1332 +++++++++++++++++++++++++++++++++++
> >> 4 files changed, 1356 insertions(+)
> >> create mode 100644 drivers/media/i2c/max9286.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index a7bb6e22d5da..99e3bf7760fd 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -10274,6 +10274,16 @@ F: Documentation/hwmon/max6697.rst
> >> F: drivers/hwmon/max6697.c
> >> F: include/linux/platform_data/max6697.h
> >>
> >> +MAX9286 QUAD GMSL DESERIALIZER DRIVER
> >> +M: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >> +M: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> +M: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >> +M: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> +L: linux-media@vger.kernel.org
> >> +S: Maintained
> >> +F: Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> >> +F: drivers/media/i2c/max9286.c
> >> +
> >> MAX9860 MONO AUDIO VOICE CODEC DRIVER
> >> M: Peter Rosin <peda@axentia.se>
> >> L: alsa-devel@alsa-project.org (moderated for non-subscribers)
> >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> >> index 3abc80373ec0..2e390f41f6da 100644
> >> --- a/drivers/media/i2c/Kconfig
> >> +++ b/drivers/media/i2c/Kconfig
> >> @@ -464,6 +464,19 @@ config VIDEO_VPX3220
> >> To compile this driver as a module, choose M here: the
> >> module will be called vpx3220.
> >>
> >> +config VIDEO_MAX9286
> >> + tristate "Maxim MAX9286 GMSL deserializer support"
> >> + depends on I2C && I2C_MUX
> >> + depends on OF
> >> + select V4L2_FWNODE
> >> + select VIDEO_V4L2_SUBDEV_API
> >> + select MEDIA_CONTROLLER
> >> + help
> >> + This driver supports the Maxim MAX9286 GMSL deserializer.
> >> +
> >> + To compile this driver as a module, choose M here: the
> >> + module will be called max9286.
> >> +
> >> comment "Video and audio decoders"
> >>
> >> config VIDEO_SAA717X
> >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> >> index 77bf7d0b691f..f0b001ee4b05 100644
> >> --- a/drivers/media/i2c/Makefile
> >> +++ b/drivers/media/i2c/Makefile
> >> @@ -117,6 +117,7 @@ obj-$(CONFIG_VIDEO_IMX274) += imx274.o
> >> obj-$(CONFIG_VIDEO_IMX290) += imx290.o
> >> obj-$(CONFIG_VIDEO_IMX319) += imx319.o
> >> obj-$(CONFIG_VIDEO_IMX355) += imx355.o
> >> +obj-$(CONFIG_VIDEO_MAX9286) += max9286.o
> >> obj-$(CONFIG_VIDEO_ST_MIPID02) += st-mipid02.o
> >>
> >> obj-$(CONFIG_SDR_MAX2175) += max2175.o
> >> diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
> >> new file mode 100644
> >> index 000000000000..481d65f2b51d
> >> --- /dev/null
> >> +++ b/drivers/media/i2c/max9286.c
> >> @@ -0,0 +1,1332 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * Maxim MAX9286 GMSL Deserializer Driver
> >> + *
> >> + * Copyright (C) 2017-2019 Jacopo Mondi
> >> + * Copyright (C) 2017-2019 Kieran Bingham
> >> + * Copyright (C) 2017-2019 Laurent Pinchart
> >> + * Copyright (C) 2017-2019 Niklas Söderlund
> >> + * Copyright (C) 2016 Renesas Electronics Corporation
> >> + * Copyright (C) 2015 Cogent Embedded, Inc.
> >> + */
> >> +
> >> +#include <linux/delay.h>
> >> +#include <linux/device.h>
> >> +#include <linux/fwnode.h>
> >> +#include <linux/gpio/consumer.h>
> >> +#include <linux/gpio/driver.h>
> >> +#include <linux/i2c.h>
> >> +#include <linux/i2c-mux.h>
> >> +#include <linux/module.h>
> >> +#include <linux/mutex.h>
> >> +#include <linux/of_graph.h>
> >> +#include <linux/regulator/consumer.h>
> >> +#include <linux/slab.h>
> >> +
> >> +#include <media/v4l2-async.h>
> >> +#include <media/v4l2-ctrls.h>
> >> +#include <media/v4l2-device.h>
> >> +#include <media/v4l2-fwnode.h>
> >> +#include <media/v4l2-subdev.h>
> >> +
> >> +/* Register 0x00 */
> >> +#define MAX9286_MSTLINKSEL_AUTO (7 << 5)
> >> +#define MAX9286_MSTLINKSEL(n) ((n) << 5)
> >> +#define MAX9286_EN_VS_GEN BIT(4)
> >> +#define MAX9286_LINKEN(n) (1 << (n))
> >> +/* Register 0x01 */
> >> +#define MAX9286_FSYNCMODE_ECU (3 << 6)
> >> +#define MAX9286_FSYNCMODE_EXT (2 << 6)
> >> +#define MAX9286_FSYNCMODE_INT_OUT (1 << 6)
> >> +#define MAX9286_FSYNCMODE_INT_HIZ (0 << 6)
> >> +#define MAX9286_GPIEN BIT(5)
> >> +#define MAX9286_ENLMO_RSTFSYNC BIT(2)
> >> +#define MAX9286_FSYNCMETH_AUTO (2 << 0)
> >> +#define MAX9286_FSYNCMETH_SEMI_AUTO (1 << 0)
> >> +#define MAX9286_FSYNCMETH_MANUAL (0 << 0)
> >> +#define MAX9286_REG_FSYNC_PERIOD_L 0x06
> >> +#define MAX9286_REG_FSYNC_PERIOD_M 0x07
> >> +#define MAX9286_REG_FSYNC_PERIOD_H 0x08
> >> +/* Register 0x0a */
> >> +#define MAX9286_FWDCCEN(n) (1 << ((n) + 4))
> >> +#define MAX9286_REVCCEN(n) (1 << (n))
> >> +/* Register 0x0c */
> >> +#define MAX9286_HVEN BIT(7)
> >> +#define MAX9286_EDC_6BIT_HAMMING (2 << 5)
> >> +#define MAX9286_EDC_6BIT_CRC (1 << 5)
> >> +#define MAX9286_EDC_1BIT_PARITY (0 << 5)
> >> +#define MAX9286_DESEL BIT(4)
> >> +#define MAX9286_INVVS BIT(3)
> >> +#define MAX9286_INVHS BIT(2)
> >> +#define MAX9286_HVSRC_D0 (2 << 0)
> >> +#define MAX9286_HVSRC_D14 (1 << 0)
> >> +#define MAX9286_HVSRC_D18 (0 << 0)
> >> +/* Register 0x0f */
> >> +#define MAX9286_0X0F_RESERVED BIT(3)
> >> +/* Register 0x12 */
> >> +#define MAX9286_CSILANECNT(n) (((n) - 1) << 6)
> >> +#define MAX9286_CSIDBL BIT(5)
> >> +#define MAX9286_DBL BIT(4)
> >> +#define MAX9286_DATATYPE_USER_8BIT (11 << 0)
> >> +#define MAX9286_DATATYPE_USER_YUV_12BIT (10 << 0)
> >> +#define MAX9286_DATATYPE_USER_24BIT (9 << 0)
> >> +#define MAX9286_DATATYPE_RAW14 (8 << 0)
> >> +#define MAX9286_DATATYPE_RAW11 (7 << 0)
> >> +#define MAX9286_DATATYPE_RAW10 (6 << 0)
> >> +#define MAX9286_DATATYPE_RAW8 (5 << 0)
> >> +#define MAX9286_DATATYPE_YUV422_10BIT (4 << 0)
> >> +#define MAX9286_DATATYPE_YUV422_8BIT (3 << 0)
> >> +#define MAX9286_DATATYPE_RGB555 (2 << 0)
> >> +#define MAX9286_DATATYPE_RGB565 (1 << 0)
> >> +#define MAX9286_DATATYPE_RGB888 (0 << 0)
> >> +/* Register 0x15 */
> >> +#define MAX9286_VC(n) ((n) << 5)
> >> +#define MAX9286_VCTYPE BIT(4)
> >> +#define MAX9286_CSIOUTEN BIT(3)
> >> +#define MAX9286_0X15_RESV (3 << 0)
> >> +/* Register 0x1b */
> >> +#define MAX9286_SWITCHIN(n) (1 << ((n) + 4))
> >> +#define MAX9286_ENEQ(n) (1 << (n))
> >> +/* Register 0x27 */
> >> +#define MAX9286_LOCKED BIT(7)
> >> +/* Register 0x31 */
> >> +#define MAX9286_FSYNC_LOCKED BIT(6)
> >> +/* Register 0x34 */
> >> +#define MAX9286_I2CLOCACK BIT(7)
> >> +#define MAX9286_I2CSLVSH_1046NS_469NS (3 << 5)
> >> +#define MAX9286_I2CSLVSH_938NS_352NS (2 << 5)
> >> +#define MAX9286_I2CSLVSH_469NS_234NS (1 << 5)
> >> +#define MAX9286_I2CSLVSH_352NS_117NS (0 << 5)
> >> +#define MAX9286_I2CMSTBT_837KBPS (7 << 2)
> >> +#define MAX9286_I2CMSTBT_533KBPS (6 << 2)
> >> +#define MAX9286_I2CMSTBT_339KBPS (5 << 2)
> >> +#define MAX9286_I2CMSTBT_173KBPS (4 << 2)
> >> +#define MAX9286_I2CMSTBT_105KBPS (3 << 2)
> >> +#define MAX9286_I2CMSTBT_84KBPS (2 << 2)
> >> +#define MAX9286_I2CMSTBT_28KBPS (1 << 2)
> >> +#define MAX9286_I2CMSTBT_8KBPS (0 << 2)
> >> +#define MAX9286_I2CSLVTO_NONE (3 << 0)
> >> +#define MAX9286_I2CSLVTO_1024US (2 << 0)
> >> +#define MAX9286_I2CSLVTO_256US (1 << 0)
> >> +#define MAX9286_I2CSLVTO_64US (0 << 0)
> >> +/* Register 0x3b */
> >> +#define MAX9286_REV_TRF(n) ((n) << 4)
> >> +#define MAX9286_REV_AMP(n) ((((n) - 30) / 10) << 1) /* in mV */
> >> +#define MAX9286_REV_AMP_X BIT(0)
> >> +/* Register 0x3f */
> >> +#define MAX9286_EN_REV_CFG BIT(6)
> >> +#define MAX9286_REV_FLEN(n) ((n) - 20)
> >> +/* Register 0x49 */
> >> +#define MAX9286_VIDEO_DETECT_MASK 0x0f
> >> +/* Register 0x69 */
> >> +#define MAX9286_LFLTBMONMASKED BIT(7)
> >> +#define MAX9286_LOCKMONMASKED BIT(6)
> >> +#define MAX9286_AUTOCOMBACKEN BIT(5)
> >> +#define MAX9286_AUTOMASKEN BIT(4)
> >> +#define MAX9286_MASKLINK(n) ((n) << 0)
> >> +
> >> +/*
> >> + * The sink and source pads are created to match the OF graph port numbers so
> >> + * that their indexes can be used interchangeably.
> >> + */
> >> +#define MAX9286_NUM_GMSL 4
> >> +#define MAX9286_N_SINKS 4
> >> +#define MAX9286_N_PADS 5
> >> +#define MAX9286_SRC_PAD 4
> >> +
> >> +struct max9286_source {
> >> + struct v4l2_async_subdev asd;
> >> + struct v4l2_subdev *sd;
> >> + struct fwnode_handle *fwnode;
> >> +};
> >> +
> >> +#define asd_to_max9286_source(_asd) \
> >> + container_of(_asd, struct max9286_source, asd)
> >> +
> >> +struct max9286_priv {
> >> + struct i2c_client *client;
> >> + struct gpio_desc *gpiod_pwdn;
> >> + struct v4l2_subdev sd;
> >> + struct media_pad pads[MAX9286_N_PADS];
> >> + struct regulator *regulator;
> >> +
> >> + struct gpio_chip gpio;
> >> + u8 gpio_state;
> >> +
> >> + struct i2c_mux_core *mux;
> >> + unsigned int mux_channel;
> >> + bool mux_open;
> >> +
> >> + struct v4l2_ctrl_handler ctrls;
> >> + struct v4l2_ctrl *pixelrate;
> >> +
> >> + struct v4l2_mbus_framefmt fmt[MAX9286_N_SINKS];
> >> +
> >> + /* Protects controls and fmt structures */
> >> + struct mutex mutex;
> >> +
> >> + unsigned int nsources;
> >> + unsigned int source_mask;
> >> + unsigned int route_mask;
> >> + unsigned int bound_sources;
> >> + unsigned int csi2_data_lanes;
> >> + struct max9286_source sources[MAX9286_NUM_GMSL];
> >> + struct v4l2_async_notifier notifier;
> >> +};
> >> +
> >
> > [...]
> >
> >> +static int max9286_register_gpio(struct max9286_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct gpio_chip *gpio = &priv->gpio;
> >> + int ret;
> >> +
> >> + static const char * const names[] = {
> >> + "GPIO0OUT",
> >> + "GPIO1OUT",
> >> + };
> >> +
> >> + /* Configure the GPIO */
> >> + gpio->label = dev_name(dev);
> >
> > So if you have more than one MAX9286 in a system, all gpiochips will appear
> > with the same name. I'd recommend to append the index to distinguish properly.
>
> Ah yes, that's a good point, and I think I've even seen that.
>
> I'll fix it now.
>
> >
> >> + gpio->parent = dev;
> >> + gpio->owner = THIS_MODULE;
> >> + gpio->of_node = dev->of_node;
> >> + gpio->ngpio = 2;
> >> + gpio->base = -1;
> >> + gpio->set = max9286_gpio_set;
> >> + gpio->get = max9286_gpio_get;
> >> + gpio->can_sleep = true;
> >> + gpio->names = names;
> >> +
> >> + /* GPIO values default to high */
> >> + priv->gpio_state = BIT(0) | BIT(1);
> >> +
> >> + ret = devm_gpiochip_add_data(dev, gpio, priv);
> >> + if (ret)
> >> + dev_err(dev, "Unable to create gpio_chip\n");
> >> +
> >> + return ret;
> >> +}
> >> +
> >
> > [...]
> >
> >> +static int max9286_parse_dt(struct max9286_priv *priv)
> >> +{
> >> + struct device *dev = &priv->client->dev;
> >> + struct device_node *i2c_mux;
> >> + struct device_node *node = NULL;
> >> + unsigned int i2c_mux_mask = 0;
> >> +
> >> + of_node_get(dev->of_node);
> >
> > Why this is needed?
>
> Hrm .. I recall adding it to solve dt reference balancing.
>
> I wish I'd added a comment at the time ... as I can't recall the details
> now.
>
I understand that it is for the refcount balancing but I certainly don't see
a need for it.
> >> + i2c_mux = of_find_node_by_name(dev->of_node, "i2c-mux");
> >> + if (!i2c_mux) {
> >> + dev_err(dev, "Failed to find i2c-mux node\n");
> >> + of_node_put(dev->of_node);
> >> + return -EINVAL;
> >> + }
> >> +
[...]
> >
> > [...]
> >
> >> +static int max9286_remove(struct i2c_client *client)
> >> +{
> >> + struct max9286_priv *priv = i2c_get_clientdata(client);
> >> +
> >> + i2c_mux_del_adapters(priv->mux);
> >> +
> >> + max9286_v4l2_unregister(priv);
> >> +
> >> + regulator_disable(priv->regulator);
> >> +
> >> + gpiod_set_value_cansleep(priv->gpiod_pwdn, 0);
> >
> > Usual power down sequence is to pull the power down gpio low and then turn off
> > the regulators. This helps in clearing up the internal state machine properly.
>
> Do you mean usual, among drivers using regulators? or usual for the max9286?
>
Usual for devices exposing shutdown gpios and powered by an external regulator.
> I have a platform which controls the regulators for the cameras
> (priv->regulator) through one of the GPIOs provided by the MAX9286.
>
> If we powerdown the max9286 first, then we will not be able to change
> the GPIO value on the gpio controlled regulator.
>
Hmm, I didn't think of this usecase. Let's keep it as it is.
Thanks,
Mani
> (Currently it doesn't expose as a gpio regulator, but just a gpio-hog -
> but thats a different matter)
>
> --
> Kieran
>
>
> >
> > Thanks,
> > Mani
> >
> >> +
> >> + max9286_cleanup_dt(priv);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static const struct of_device_id max9286_dt_ids[] = {
> >> + { .compatible = "maxim,max9286" },
> >> + {},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, max9286_dt_ids);
> >> +
> >> +static struct i2c_driver max9286_i2c_driver = {
> >> + .driver = {
> >> + .name = "max9286",
> >> + .of_match_table = of_match_ptr(max9286_dt_ids),
> >> + },
> >> + .probe_new = max9286_probe,
> >> + .remove = max9286_remove,
> >> +};
> >> +
> >> +module_i2c_driver(max9286_i2c_driver);
> >> +
> >> +MODULE_DESCRIPTION("Maxim MAX9286 GMSL Deserializer Driver");
> >> +MODULE_AUTHOR("Jacopo Mondi, Kieran Bingham, Laurent Pinchart, Niklas Söderlund, Vladimir Barinov");
> >> +MODULE_LICENSE("GPL");
> >> --
> >> 2.25.1
> >>
>
^ permalink raw reply
* Re: [PATCH] fixup! dt-bindings: media: i2c: Add bindings for Maxima Integrated MAX9286
From: Jacopo Mondi @ 2020-05-14 10:20 UTC (permalink / raw)
To: Kieran Bingham
Cc: Jacopo Mondi, devicetree, linux-media, Rob Herring,
linux-renesas-soc, laurent.pinchart+renesas, niklas.soderlund,
Hans Verkuil, sakari.ailus, mchehab, Manivannan Sadhasivam,
Hyun Kwon
In-Reply-To: <bd648891-67c2-098b-3a0e-72bf4e938535@ideasonboard.com>
Hi Kieran,
On Thu, May 14, 2020 at 10:43:10AM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 13/05/2020 13:54, Jacopo Mondi wrote:
> > Subject: [PATCH] fixup! dt-bindings: media: i2c: Add bindings for Maxima Integrated MAX9286
> >
> > Temporary fixup to ease review. To be squashed into v10 if accepted.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >
> > While fixing Rob's reported bug on v9 I realized thanks to a
> > dt_binding_check warning that the i2c-mux child nodes where under-specified.
> >
> > This fixup epxands the i2c-mux child nodes description and updates the
> > example to match our currently-out-of-tree DTS files for GMSL platforms.
> >
> > dt_binding_check still reports a warning:
> >
> > Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: camera@51: reg: [[81, 97]] is too short
> > Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: camera@52: reg: [[82, 98]] is too short
> > Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: camera@53: reg: [[83, 99]] is too short
> > Documentation/devicetree/bindings/media/i2c/maxim,max9286.example.dt.yaml: camera@54: reg: [[84, 100]] is too short
>
> Is too short? That seems odd. We accept 1, 2, or 3 values in the reg
> (for the serializer, camera, mcu addresses) but those values are
> specific to the camera device, and are managed by that driver and
> shouldn't be validated here.
I think it's expected them to be validated against this schema, not
sure what's the warning about though.
>
>
> >
> > Which I was not able to silence.
> > ---
> > .../bindings/media/i2c/maxim,max9286.yaml | 86 +++++++++++++++++--
> > 1 file changed, 77 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > index f9d3e5712c59..d9bd19caed2f 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/maxim,max9286.yaml
> > @@ -130,9 +130,7 @@ properties:
> > description: |
> > Each GMSL link is modelled as a child bus of an i2c bus
> > multiplexer/switch, in accordance with bindings described in
> > - Documentation/devicetree/bindings/i2c/i2c-mux.txt. The serializer
> > - device on the remote end of the GMSL link shall be modelled as a child
> > - node of the corresponding I2C bus.
> > + Documentation/devicetree/bindings/i2c/i2c-mux.txt.
> >
> > properties:
> > '#address-cells':
> > @@ -141,7 +139,74 @@ properties:
> > '#size-cells':
> > const: 0
> >
> > - additionalProperties: false
> > + patternProperties:
> > + "^i2c@[0-3]$":
> > + type: object
> > + description: |
> > + Child node of the i2c bus multiplexer which represents a GMSL link.
> > + Each serializer device on the GMSL link remote end is represented with
> > + an i2c-mux child node. The MAX9286 chip supports up to 4 GMSL
> > + channels.
> > +
> > + properties:
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + reg:
> > + description: The index of the GMSL channel.
> > + maxItems: 1
> > +
> > + patternProperties:
> > + "^camera@[0-9]+":
>
> This value after @ represents a non 0x prefixed hex value, where your
> regex will only match numerical values.
>
> "^camera@[a-f0-9]+":
>
>
>
> is likely therefore required.
>
Uh, correct
> I see other uses of this patternProperties also terminate the regex with
> a $ (end of line) but that confuses me, as the node is often presented
> with the opening brace after the name and identifier:
>
> (i.e.)
> camera@52 {
>
> But perhaps the validator extracts the node name and reg and matches on
> that directly in which case it would be
>
> "^camera@[a-f0-9]+$":
I've initially used '$' as well then removed it for the same reason.
MAybe the patter is only matched against the node name, leaving out the
rest of the line.
>
>
>
> > + type: object
> > + description: |
> > + The remote camera device, composed by a GMSL serializer and a
> > + connected video source.
> > +
> > + properties:
> > + compatible:
> > + description: The remote device compatible string.
> > +
> > + reg:
> > + description: |
> > + The I2C addresses to be assigned to the remote devices through
> > + address reprogramming. The number of entries depends on the
> > + requirements of the currently connected remote device.
> > +
> > + port:
> > + type: object
> > +
> > + properties:
> > + endpoint:
> > + type: object
> > +
> > + properties:
> > + remote-endpoint:
> > + description: phandle to the MAX9286 sink endpoint.
> > +
> > + required:
> > + - remote-endpoint
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - endpoint
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - compatible
> > + - reg
> > + - port
> > +
> > + additionalProperties: false
> > +
> > + additionalProperties: false
> > +
> > + additionalProperties: false
> >
>
>
> Wow, is it required to specify additionalProperties: false quite so many
> times?
I -think- so, it's one per section, and unless it's by default it
makes sense to have it there ?
>
>
> > required:
> > - compatible
> > @@ -220,11 +285,11 @@ examples:
> > i2c@0 {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > -
> > reg = <0>;
> >
> > camera@51 {
> > - reg = <0x51>;
> > + compatible = "imi,rdacm20";
> > + reg = <0x51 0x61>;
> >
> > port {
> > rdacm20_out0: endpoint {
> > @@ -241,7 +306,8 @@ examples:
> > reg = <1>;
> >
> > camera@52 {
> > - reg = <0x52>;
> > + compatible = "imi,rdacm20";
> > + reg = <0x52 0x62>;
> >
> > port {
> > rdacm20_out1: endpoint {
> > @@ -257,7 +323,8 @@ examples:
> > reg = <2>;
> >
> > camera@53 {
> > - reg = <0x53>;
> > + compatible = "imi,rdacm20";
> > + reg = <0x53 0x63>;
> >
> > port {
> > rdacm20_out2: endpoint {
> > @@ -273,7 +340,8 @@ examples:
> > reg = <3>;
> >
> > camera@54 {
> > - reg = <0x54>;
> > + compatible = "imi,rdacm20";
> > + reg = <0x54 0x64>;
> >
> > port {
> > rdacm20_out3: endpoint {
> > --
> > 2.26.2
> >
>
^ permalink raw reply
* Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller
From: Felipe Balbi @ 2020-05-14 10:21 UTC (permalink / raw)
To: Roger Quadros, Chunfeng Yun
Cc: robh+dt, vigneshr, linux-usb, devicetree, linux-kernel
In-Reply-To: <baaaf89c-4baf-4218-e22e-53cd1a64ec02@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
Roger Quadros <rogerq@ti.com> writes:
> On 14/05/2020 04:37, Chunfeng Yun wrote:
>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>> requires SERDES0 to be powered on before USB.
>>>
>>> We need to power up SERDES0 power domain and hold it on
>>> throughout the reset, init, power on sequence.
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>> drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>> 1 file changed, 46 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>> index 1e14a6f4884b..46d46f3507fc 100644
>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/dma-mapping.h>
>>> #include <linux/io.h>
>>> #include <linux/of_platform.h>
>>> +#include <linux/phy/phy.h>
>>> #include <linux/pm_runtime.h>
>>>
>>> /* USBSS register offsets */
>>> @@ -34,6 +35,7 @@
>>> struct dwc3_keystone {
>>> struct device *dev;
>>> void __iomem *usbss;
>>> + struct phy *usb3_phy;
>>> };
>>>
>>> static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>> if (IS_ERR(kdwc->usbss))
>>> return PTR_ERR(kdwc->usbss);
>>>
>>> - pm_runtime_enable(kdwc->dev);
>>> + /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>> + kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>> Use devm_phy_optional_get() instead?
>
> Indeed, it seems better suited.
patches 1 and 2 are in testing/next
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/5] drm/panel: Add ilitek ili9341 driver
From: dillon min @ 2020-05-14 10:22 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Maxime Coquelin, Alexandre TORGUE,
thierry.reding@gmail.com, Sam Ravnborg, Dave Airlie,
Daniel Vetter, Michael Turquette, Stephen Boyd,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-stm32, Linux ARM, linux-kernel@vger.kernel.org,
open list:DRM PANEL DRIVERS, linux-clk
In-Reply-To: <CACRpkdbZoMDC-D12CByKJUZbu4shqixC=QrKwJUd8x=nyK7seQ@mail.gmail.com>
Hi Linus,
Thanks for reviewing.
On Thu, May 14, 2020 at 4:14 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Dillon,
>
> thanks for your patch! Overall this looks like a good start.
>
> On Tue, May 12, 2020 at 9:04 AM <dillon.minfei@gmail.com> wrote:
>
> > #define ILI9341_SLEEP_OUT 0x11 /* Sleep out register */
>
> This is not a register, also just use MIPI_DCS_EXIT_SLEEP_MODE
> in the code.
>
Yes, i will try to reuse MIPI_DCS_xxx.
> > +#define ILI9341_DFC 0xb6 /* Display Function Control
> > + * register
> > + */
>
> This commenting style doesn't work, either just put it after /* the define */
> and don't care if the line gets a bit long and checkpatch complains,
> or
>
> /*
> * Put it above the define like this
> */
> #define FOO 0x00
>
Ok, will change this comments.
> > +/**
> > + * struct ili9341_config - the system specific ILI9341 configuration
>
> Nice with this per-system config, it makes the driver easy to maintain
> for new users.
>
Yes, will try to move more system related configurations to this part,
instead of hard code.
> > +static int ili9341_dpi_init(struct ili9341 *ili)
> > +{
> > + ili9341_command(ili, 0xca, 0xc3, 0x08, 0x50);
>
> This stuff is a bit hard to understand, don't you think?
>
> But given that register 0xCA seems undocumented I don't
> know if there is anything more you can do, so it is OK
> I suppose.
>
> > + ili9341_command(ili, ILI9341_POWERB, 0x00, 0xc1, 0x30);
>
> This command is described in the manual page 196.
> Version: V1.11
> Document No.: ILI9341_DS_V1.11.pdf
> https://dflund.se/~triad/ILI9341_v1.11.pdf
>
Yes, "ili9341_command(ili, 0xca, 0xc3, 0x08, 0x50);" i ported from st's sdk.
will use ILI9341_XXX to replace these magic numbers
> And this goes for all the below commands. Please add some more defines
> from the datasheet and have less magic numbers in the driver.
>
> > + ili9341_command(ili, ILI9341_POWER_SEQ, 0x64, 0x03, 0x12, 0x81);
> > + ili9341_command(ili, ILI9341_DTCA, 0x85, 0x00, 0x78);
> > + ili9341_command(ili, ILI9341_POWERA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> > + ili9341_command(ili, ILI9341_PRC, 0x20);
> > + ili9341_command(ili, ILI9341_DTCB, 0x00, 0x00);
> > + ili9341_command(ili, ILI9341_FRC, 0x00, 0x1b);
> > + ili9341_command(ili, ILI9341_DFC, 0x0a, 0xa2);
> > + ili9341_command(ili, ILI9341_POWER1, 0x10);
> > + ili9341_command(ili, ILI9341_POWER2, 0x10);
> > + ili9341_command(ili, ILI9341_VCOM1, 0x45, 0x15);
> > + ili9341_command(ili, ILI9341_VCOM2, 0x90);
> > + ili9341_command(ili, ILI9341_MAC, 0xc8);
> > + ili9341_command(ili, ILI9341_3GAMMA_EN, 0x00);
> > + ili9341_command(ili, ILI9341_RGB_INTERFACE, 0xc2);
> > + ili9341_command(ili, ILI9341_DFC, 0x0a, 0xa7, 0x27, 0x04);
> > + ili9341_command(ili, ILI9341_COLUMN_ADDR, 0x00, 0x00, 0x00, 0xef);
> > + ili9341_command(ili, ILI9341_PAGE_ADDR, 0x00, 0x00, 0x01, 0x3f);
> > + ili9341_command(ili, ILI9341_INTERFACE, 0x01, 0x00, 0x06);
> > + if (ili->input == ILI9341_INPUT_PRGB_18_BITS)
> > + ili9341_command(ili, ILI9341_PIXEL_FORMAT, 0x66);
> > + else
> > + ili9341_command(ili, ILI9341_PIXEL_FORMAT, 0x56);
> > + ili9341_command(ili, ILI9341_GRAM);
> > + msleep(200);
>
> I think some of the above should not be hard coded but should instead
> be stored in fields in struct ili9341_config. I know it can be a bit
> tedious but it makes things much more clear.
>
Ok, will go deeper to find out some register configuration move to
system config like rgb bus 16/18 bits
> > + ili9341_command(ili, ILI9341_GAMMA, 0x01);
> > + ili9341_command(ili, ILI9341_PGAMMA, 0x0f, 0x29, 0x24, 0x0c, 0x0e,
> > + 0x09, 0x4e, 0x78, 0x3c, 0x09,
> > + 0x13, 0x05, 0x17, 0x11, 0x00);
> > + ili9341_command(ili, ILI9341_NGAMMA, 0x00, 0x16, 0x1b, 0x04, 0x11,
> > + 0x07, 0x31, 0x33, 0x42, 0x05,
> > + 0x0c, 0x0a, 0x28, 0x2f, 0x0f);
>
> This should definately be in ili9341_config, as it is a screen property.
>
> In the long run I would like these panels to support setting gamma
> from userspace etc but it is a big tedious work to get that right
> so hard-coding a default per-variant is fine.
>
ok, will refer to panel-ilitek-ili9322 and panel-novatek-nt35510 driver.
> You can check in e.g. panel-novatek-nt35510.c how I encoded
> such sequences in per-variant data.
>
> > +static int ili9341_dpi_power_off(struct ili9341 *ili)
> > +{
> > + /* Disable power */
> > + if (!IS_ERR(ili->vcc))
> > + return regulator_disable(ili->vcc);
> > +
> > + return 0;
> > +}
>
> Usually you should also assert RESET when disabling
> power.
>
ok
> > +/* This is the only mode listed for parallel RGB in the datasheet */
> > +static const struct drm_display_mode rgb_240x320_mode = {
> > + .clock = 6100,
> > + .hdisplay = 240,
> > + .hsync_start = 240 + 10,
> > + .hsync_end = 240 + 10 + 10,
> > + .htotal = 240 + 10 + 10 + 20,
> > + .vdisplay = 320,
> > + .vsync_start = 320 + 4,
> > + .vsync_end = 320 + 4 + 2,
> > + .vtotal = 320 + 4 + 2 + 2,
> > + .vrefresh = 60,
> > + .flags = 0,
> > + .width_mm = 65,
> > + .height_mm = 50,
>
> The width and height should certainly be om the ili9341_config
> as it is a per-panel property. You can just fill in in in
> the below .get_modes() function. Or assign the whole
> mode as part of the ili9341_config if that is easier.
>
ok, lcd timing part will move to ili9341 config
> > + return drm_panel_add(&ili->panel);
> > +}
> > +
> > +
> > +
>
> Surplus whitespace here.
ok, will delete it.
>
> > + mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_OFF);
> > +
> > + mipi_dbi_command(dbi, ILI9341_POWERB, 0x00, 0xc1, 0x30);
> > + mipi_dbi_command(dbi, ILI9341_POWER_SEQ, 0x64, 0x03, 0x12, 0x81);
>
> Some of these are just copies of the above init sequence, so it makes
> even more sense to just have these settings stored in
> ili9341_config.
ok.
>
> > + mipi_dbi_command(dbi, ILI9341_DTCA, 0x85, 0x00, 0x78);
> > + mipi_dbi_command(dbi, ILI9341_POWERA, 0x39, 0x2c, 0x00, 0x34, 0x02);
> > + mipi_dbi_command(dbi, ILI9341_PRC, 0x20);
> > + mipi_dbi_command(dbi, ILI9341_DTCB, 0x00, 0x00);
> > +
> > + /* Power Control */
> > + mipi_dbi_command(dbi, ILI9341_POWER1, 0x23);
> > + mipi_dbi_command(dbi, ILI9341_POWER2, 0x10);
> > + /* VCOM */
> > + mipi_dbi_command(dbi, ILI9341_VCOM1, 0x3e, 0x28);
> > + mipi_dbi_command(dbi, ILI9341_VCOM2, 0x86);
> > +
> > + /* Memory Access Control */
> > + mipi_dbi_command(dbi, MIPI_DCS_SET_PIXEL_FORMAT,
> > + MIPI_DCS_PIXEL_FMT_16BIT);
> > +
> > + /* Frame Rate */
> > + mipi_dbi_command(dbi, ILI9341_FRC, 0x00, 0x1b);
> > +
> > + /* Gamma */
> > + mipi_dbi_command(dbi, ILI9341_3GAMMA_EN, 0x00);
> > + mipi_dbi_command(dbi, MIPI_DCS_SET_GAMMA_CURVE, 0x01);
> > + mipi_dbi_command(dbi, ILI9341_PGAMMA,
> > + 0x0f, 0x31, 0x2b, 0x0c, 0x0e, 0x08, 0x4e, 0xf1,
> > + 0x37, 0x07, 0x10, 0x03, 0x0e, 0x09, 0x00);
> > + mipi_dbi_command(dbi, ILI9341_NGAMMA,
> > + 0x00, 0x0e, 0x14, 0x03, 0x11, 0x07, 0x31, 0xc1,
> > + 0x48, 0x08, 0x0f, 0x0c, 0x31, 0x36, 0x0f);
>
> It seems to be copies of the stuff above, but why is there a different
> gamma if you use DBI?
for dbi interface, currently i just copy the code from tiny/ili9341.c.
as so many boards use this driver now, like raspberry pi, etc
i'm afraid it's will not work after modification. so, just leave the
original code there.
>
> I suspect only one of them is really needed and it is not even
> necessary to set if up in two places.
>
as i know, dbi interface use spi to transfer video data and register
set to panel.
but dpi use rgb bus transfer video data, spi set register, they are
two different type for drm.
i can't register two different interface into drm at the same time.
so, i use two path.
for code management, they have some common part, like register init.
power on/off process. i will try to reuse most common functions. to make code
easier to be understand.
anther question:
is there any panel driver have dbi & dpi or dpi & dsi supported? which
i mean support
two different panel interface in one driver.
thanks
> > +out_enable:
> > + switch (dbidev->rotation) {
> > + default:
> > + addr_mode = ILI9341_MADCTL_MX;
> > + break;> +out_enable:
> > + switch (dbidev->rotation) {
> > + default:
> > + addr_mode = ILI9341_MADCTL_MX;
> > + break;
> > + case 90:
> > + addr_mode = ILI9341_MADCTL_MV;
> > + break;
> > + case 180:
> > + addr_mode = ILI9341_MADCTL_MY;
> > + break;
> > + case 270:
> > + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> > + ILI9341_MADCTL_MX;
> > + break;
> > + }
> > + addr_mode |= ILI9341_MADCTL_BGR;
> > + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
> > + mipi_dbi_enable_flush(dbidev, crtc_state, plane_state);
> > + DRM_DEBUG_KMS("initialized display serial interface\n");
> > +out_exit:
> > + drm_dev_exit(idx);
> > +}
> > +
>
> > + case 90:
> > + addr_mode = ILI9341_MADCTL_MV;
> > + break;
> > + case 180:
> > + addr_mode = ILI9341_MADCTL_MY;
> > + break;
> > + case 270:
> > + addr_mode = ILI9341_MADCTL_MV | ILI9341_MADCTL_MY |
> > + ILI9341_MADCTL_MX;
> > + break;
> > + }
> > + addr_mode |= ILI9341_MADCTL_BGR;
> > + mipi_dbi_command(dbi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
>
> Since you use MIPI_DCS_* define here, check if this applies
> to more of the commands above so you don't need custom
> defines for them. e.g.
> ILI9341_SLEEP_OUT 0x11 = MIPI_DCS_EXIT_SLEEP_MODE
> and that isn't even a register right, it is just a command?
> (Noted in the beginning.)
>
ok, will try to reuse MIPI_DCS_xxx
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 0/4] ADD interconnect support for Qualcomm DWC3 driver
From: Felipe Balbi @ 2020-05-14 10:49 UTC (permalink / raw)
To: Sandeep Maheswaram (Temp), Matthias Kaehlcke
Cc: Andy Gross, Bjorn Andersson, Greg Kroah-Hartman, Rob Herring,
Mark Rutland, Stephen Boyd, Doug Anderson, linux-arm-msm,
linux-usb, devicetree, linux-kernel, Manu Gautam,
Chandana Kishori Chiluveru
In-Reply-To: <a119cf75-8bda-f380-8249-173fa426279c@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 151 bytes --]
"Sandeep Maheswaram (Temp)" <sanm@codeaurora.org> writes:
> Hi Felipe,
>
> Any update about landing this series.
in my tree now
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* [PATCH 1/6] arm64: dts: qcom: sc7180: Add interconnect bindings for GPU
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
This patch adds the interconnect bindings to the GPU node. This enables
the GPU->DDR path bandwidth voting.
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index b46ee78..0ce9921 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1384,6 +1384,8 @@
operating-points-v2 = <&gpu_opp_table>;
qcom,gmu = <&gmu>;
+ interconnects = <&gem_noc MASTER_GFX3D &mc_virt SLAVE_EBI1>;
+
gpu_opp_table: opp-table {
compatible = "operating-points-v2";
--
2.7.4
^ permalink raw reply related
* [PATCH 0/6] Add support for GPU DDR BW scaling
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
This is a rework of my previous series [1], but this time based on the bindings
from Georgi [2] + a few fixes which look to be fixed in v8 of Georgi's series
[3]. The work is based on the chromeOS tip.
[1]: https://patchwork.freedesktop.org/series/75291/
[2]: https://lore.kernel.org/patchwork/cover/1230626/
[3]: https://lore.kernel.org/patchwork/cover/1240687/
Sharat Masetty (5):
arm64: dts: qcom: sc7180: Add interconnect bindings for GPU
arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp
drm: msm: a6xx: send opp instead of a frequency
drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
dt-bindings: drm/msm/gpu: Document gpu opp table
Sibi Sankar (1):
OPP: Add and export helper to set bandwidth
.../devicetree/bindings/display/msm/gpu.txt | 28 +++++++++
arch/arm64/boot/dts/qcom/sc7180.dtsi | 9 +++
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 68 +++++++++++-----------
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
drivers/gpu/drm/msm/msm_gpu.c | 3 +-
drivers/gpu/drm/msm/msm_gpu.h | 3 +-
drivers/opp/core.c | 43 ++++++++++++++
include/linux/pm_opp.h | 6 ++
8 files changed, 125 insertions(+), 37 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 2/6] arm64: dts: qcom: sc7180: Add opp-peak-kBps to GPU opp
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
Add opp-peak-kBps bindings to the GPU opp table, listing the peak
GPU -> DDR bandwidth requirement for each opp level. This will be
used to scale the DDR bandwidth along with the GPU frequency dynamically.
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sc7180.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 0ce9921..89f7767 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -1392,36 +1392,43 @@
opp-800000000 {
opp-hz = /bits/ 64 <800000000>;
opp-level = <RPMH_REGULATOR_LEVEL_TURBO>;
+ opp-peak-kBps = <8532000>;
};
opp-650000000 {
opp-hz = /bits/ 64 <650000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM_L1>;
+ opp-peak-kBps = <7216000>;
};
opp-565000000 {
opp-hz = /bits/ 64 <565000000>;
opp-level = <RPMH_REGULATOR_LEVEL_NOM>;
+ opp-peak-kBps = <5412000>;
};
opp-430000000 {
opp-hz = /bits/ 64 <430000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+ opp-peak-kBps = <5412000>;
};
opp-355000000 {
opp-hz = /bits/ 64 <355000000>;
opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+ opp-peak-kBps = <3072000>;
};
opp-267000000 {
opp-hz = /bits/ 64 <267000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ opp-peak-kBps = <3072000>;
};
opp-180000000 {
opp-hz = /bits/ 64 <180000000>;
opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+ opp-peak-kBps = <1804000>;
};
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH 3/6] OPP: Add and export helper to set bandwidth
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sibi Sankar, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
From: Sibi Sankar <sibis@codeaurora.org>
Add and export 'dev_pm_opp_set_bw' to set the bandwidth
levels associated with an OPP for a given frequency.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
drivers/opp/core.c | 43 +++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 6 ++++++
2 files changed, 49 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index f42b7c4..0f34077 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -828,6 +828,49 @@ static int _set_required_opps(struct device *dev,
}
/**
+ * dev_pm_opp_set_bw() - sets bandwidth levels corresponding to an available opp
+ * @dev: device for which we do this operation
+ * @opp: opp based on which the bandwidth levels are to be configured
+ *
+ * This configures the bandwidth to the levels specified
+ * by the OPP.
+ *
+ * Return: 0 on success or a negative error value.
+ */
+int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp)
+{
+ struct opp_table *opp_table;
+ int ret = -EINVAL;
+ int i;
+
+ if (IS_ERR_OR_NULL(opp) || !opp->available) {
+ dev_err(dev, "%s: Invalid parameters\n", __func__);
+ return -EINVAL;
+ }
+
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ dev_err(dev, "%s: device opp table doesn't exist\n", __func__);
+ return PTR_ERR(opp_table);
+ }
+
+ if (opp_table->paths) {
+ for (i = 0; i < opp_table->path_count; i++) {
+ ret = icc_set_bw(opp_table->paths[i],
+ opp->bandwidth[i].avg,
+ opp->bandwidth[i].peak);
+ if (ret)
+ dev_err(dev, "Failed to set bandwidth[%d]: %d\n",
+ i, ret);
+ }
+ }
+
+ dev_pm_opp_put_opp_table(opp_table);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_bw);
+
+/**
* dev_pm_opp_set_rate() - Configure new OPP based on frequency
* @dev: device for which we do this operation
* @target_freq: frequency to achieve
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 76f8c6b..04f7fda 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -156,6 +156,7 @@ struct dev_pm_opp *dev_pm_opp_xlate_opp(struct opp_table *src_table,
struct opp_table *dst_table,
struct dev_pm_opp *src_opp);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
+int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
void dev_pm_opp_remove_table(struct device *dev);
@@ -354,6 +355,11 @@ static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_f
return -ENOTSUPP;
}
+static inline int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp)
+{
+ return -ENOTSUPP;
+}
+
static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask)
{
return -ENOTSUPP;
--
2.7.4
^ permalink raw reply related
* [PATCH 6/6] dt-bindings: drm/msm/gpu: Document gpu opp table
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
Update documentation to list the gpu opp table bindings including the
newly added "opp-peak-kBps" needed for GPU-DDR bandwidth scaling.
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
.../devicetree/bindings/display/msm/gpu.txt | 28 ++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 70025cb..48bd4ab 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -79,6 +79,34 @@ Example a6xx (with GMU):
interconnects = <&rsc_hlos MASTER_GFX3D &rsc_hlos SLAVE_EBI1>;
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-430000000 {
+ opp-hz = /bits/ 64 <430000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_SVS_L1>;
+ opp-peak-kBps = <5412000>;
+ };
+
+ opp-355000000 {
+ opp-hz = /bits/ 64 <355000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_SVS>;
+ opp-peak-kBps = <3072000>;
+ };
+
+ opp-267000000 {
+ opp-hz = /bits/ 64 <267000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
+ opp-peak-kBps = <3072000>;
+ };
+
+ opp-180000000 {
+ opp-hz = /bits/ 64 <180000000>;
+ opp-level = <RPMH_REGULATOR_LEVEL_MIN_SVS>;
+ opp-peak-kBps = <1804000>;
+ };
+ };
+
qcom,gmu = <&gmu>;
zap-shader {
--
2.7.4
^ permalink raw reply related
* [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
This patches replaces the previously used static DDR vote and uses
dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
GPU frequency.
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 2d8124b..79433d3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
gmu->freq = gmu->gpu_freqs[perf_index];
- /*
- * Eventually we will want to scale the path vote with the frequency but
- * for now leave it at max so that the performance is nominal.
- */
- icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
+ dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
}
unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
--
2.7.4
^ permalink raw reply related
* [PATCH 4/6] drm: msm: a6xx: send opp instead of a frequency
From: Sharat Masetty @ 2020-05-14 10:54 UTC (permalink / raw)
To: freedreno, devicetree
Cc: dri-devel, linux-arm-msm, linux-kernel, jcrouse, georgi.djakov,
mka, Sharat Masetty
In-Reply-To: <1589453659-27581-1-git-send-email-smasetty@codeaurora.org>
This patch changes the plumbing to send the devfreq recommended opp rather
than the frequency. Also consolidate and rearrange the code in a6xx to set
the GPU frequency and the icc vote in preparation for the upcoming
changes for GPU->DDR scaling votes.
Signed-off-by: Sharat Masetty <smasetty@codeaurora.org>
---
drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 62 +++++++++++++++++++----------------
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 2 +-
drivers/gpu/drm/msm/msm_gpu.c | 3 +-
drivers/gpu/drm/msm/msm_gpu.h | 3 +-
4 files changed, 38 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 748cd37..2d8124b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -100,17 +100,30 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF));
}
-static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
+void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
{
- struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
- struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
- struct msm_gpu *gpu = &adreno_gpu->base;
- int ret;
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
+ u32 perf_index;
+ unsigned long gpu_freq;
+ int ret = 0;
+
+ gpu_freq = dev_pm_opp_get_freq(opp);
+
+ if (gpu_freq == gmu->freq)
+ return;
+
+ for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
+ if (gpu_freq == gmu->gpu_freqs[perf_index])
+ break;
+
+ gmu->current_perf_index = perf_index;
gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
- ((3 & 0xf) << 28) | index);
+ ((3 & 0xf) << 28) | perf_index);
/*
* Send an invalid index as a vote for the bus bandwidth and let the
@@ -126,7 +139,7 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
if (ret)
dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
- gmu->freq = gmu->gpu_freqs[index];
+ gmu->freq = gmu->gpu_freqs[perf_index];
/*
* Eventually we will want to scale the path vote with the frequency but
@@ -135,25 +148,6 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
}
-void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
-{
- struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
- struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
- struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
- u32 perf_index = 0;
-
- if (freq == gmu->freq)
- return;
-
- for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
- if (freq == gmu->gpu_freqs[perf_index])
- break;
-
- gmu->current_perf_index = perf_index;
-
- __a6xx_gmu_set_freq(gmu, perf_index);
-}
-
unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -708,6 +702,19 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
a6xx_gmu_rpmh_off(gmu);
}
+static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
+{
+ struct dev_pm_opp *gpu_opp;
+ unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index];
+
+ gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true);
+ if (IS_ERR_OR_NULL(gpu_opp))
+ return;
+
+ a6xx_gmu_set_freq(gpu, gpu_opp);
+ dev_pm_opp_put(gpu_opp);
+}
+
int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
{
struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
@@ -759,8 +766,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK);
enable_irq(gmu->hfi_irq);
- /* Set the GPU to the current freq */
- __a6xx_gmu_set_freq(gmu, gmu->current_perf_index);
+ a6xx_gmu_set_initial_freq(gpu, gmu);
/*
* "enable" the GX power domain which won't actually do anything but it
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index 7239b8b..03ba60d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -63,7 +63,7 @@ void a6xx_gmu_clear_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state);
int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node);
void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu);
-void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq);
+void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp);
unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu);
void a6xx_show(struct msm_gpu *gpu, struct msm_gpu_state *state,
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 18f3a5c..f703da0 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -13,7 +13,6 @@
#include <generated/utsrelease.h>
#include <linux/string_helpers.h>
-#include <linux/pm_opp.h>
#include <linux/devfreq.h>
#include <linux/devcoredump.h>
#include <linux/sched/task.h>
@@ -34,7 +33,7 @@ static int msm_devfreq_target(struct device *dev, unsigned long *freq,
return PTR_ERR(opp);
if (gpu->funcs->gpu_set_freq)
- gpu->funcs->gpu_set_freq(gpu, (u64)*freq);
+ gpu->funcs->gpu_set_freq(gpu, opp);
else
clk_set_rate(gpu->core_clk, *freq);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index ab8f0f9c..cf0dc6d 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -9,6 +9,7 @@
#include <linux/clk.h>
#include <linux/interconnect.h>
+#include <linux/pm_opp.h>
#include <linux/regulator/consumer.h>
#include "msm_drv.h"
@@ -63,7 +64,7 @@ struct msm_gpu_funcs {
struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
int (*gpu_state_put)(struct msm_gpu_state *state);
unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
- void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq);
+ void (*gpu_set_freq)(struct msm_gpu *gpu, struct dev_pm_opp *opp);
};
struct msm_gpu {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v8 1/8] tpm: tpm_tis: Make implementation of read16, read32 and write32 optional
From: Jarkko Sakkinen @ 2020-05-14 11:26 UTC (permalink / raw)
To: amirmizi6, Eyal.Cohen, oshrialkoby85, alexander.steffen, robh+dt,
benoit.houyere, mark.rutland, peterhuewe, christophe-h.richard,
jgg, arnd, gregkh
Cc: devicetree, linux-kernel, linux-integrity, oshri.alkoby,
tmaimon77, gcwilson, kgoldman, Dan.Morav, oren.tanami,
shmulik.hager, amir.mizinski
In-Reply-To: <20200512141431.83833-2-amirmizi6@gmail.com>
On Tue, 2020-05-12 at 17:14 +0300, amirmizi6@gmail.com wrote:
> From: Amir Mizinski <amirmizi6@gmail.com>
>
> Only tpm_tis can use memory-mapped I/O, which is truly mapped into
> the kernel's memory space. Therefore, using ioread16/ioread32/iowrite32
> turns into a straightforward pointer dereference.
> Every other driver requires more complicated operations to read more than
> one byte at a time and will just fall back to read_bytes/write_bytes.
> Therefore, move this common code out of tpm_tis_spi and into tpm_tis_core
> so that it is used automatically when low-level drivers do not implement
> the specialized methods.
>
> Co-developed-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Alexander Steffen <Alexander.Steffen@infineon.com>
> Signed-off-by: Amir Mizinski <amirmizi6@gmail.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
OK, so I applied this one:
http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/fa05dc792ea02043f3c21467cb4485a38ac19bdf
I.e. no need to carry this one any more in the series. Sorry that
I haven't done this before (should have).
/Jarkko
^ permalink raw reply
* Re: [v2 3/4] regulator: qcom: Add labibb driver
From: Sumit Semwal @ 2020-05-14 11:27 UTC (permalink / raw)
To: Mark Brown
Cc: agross, Bjorn Andersson, lgirdwood, robh+dt, Nisha Kumari,
linux-arm-msm, LKML, devicetree, kgunda, Rajendra Nayak
In-Reply-To: <20200511103937.GC8216@sirena.org.uk>
Hello Mark,
Thank you for your review comments!
On Mon, 11 May 2020 at 16:09, Mark Brown <broonie@kernel.org> wrote:
>
> On Sat, May 09, 2020 at 02:11:59AM +0530, Sumit Semwal wrote:
>
> > + ret = regmap_bulk_read(reg->regmap, reg->base +
> > + REG_LABIBB_STATUS1, &val, 1);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> > + return ret;
> > + }
>
> Why a bulk read of a single register?
Right, will change.
>
> > +static int _check_enabled_with_retries(struct regulator_dev *rdev,
> > + int retries, int enabled)
> > +{
>
> This is not retrying, this is polling to see if the regulator actually
> enabled.
Yes, will update accordingly.
>
> > +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> > +{
>
> > + ret = _check_enabled_with_retries(rdev, retries, 1);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "retries exhausted: enable %s regulator\n",
> > + reg->desc.name);
> > + return ret;
> > + }
>
> If this is useful factor it out into a helper or the core, other devices
> also have status bits saying if the regulator is enabled. It looks like
> this may be mainly trying to open code something like enable_time, with
> possibly some issues where the time taken to enable varies a lot.
>
Makes sense; I am not terribly familiar with the regulator core and
helpers, so let me look and refactor accordingly.
> > + if (ret)
> > + return 0;
> > +
> > +
> > + dev_err(reg->dev, "Can't enable %s\n", reg->desc.name);
> > + return -EINVAL;
>
> Return the actual error code (the logic here is quite convoluted).
Will try to simplify.
>
> > + ret = regulator_disable_regmap(rdev);
> > +
> > + if (ret < 0) {
>
> You have lots of blank lines between operations and checking their
> return codes?
>
will correct that.
> > + ret = _check_enabled_with_retries(rdev, retries, 0);
> > + if (ret < 0) {
> > + dev_err(reg->dev, "retries exhausted: disable %s regulator\n",
> > + reg->desc.name);
> > + return ret;
> > + }
>
> Similarly to the enable path, but is this one about off_on_delay rather
> than enable_time?
Got it. Let me look deeper.
>
> > + if (reg_data->type == QCOM_LAB_TYPE) {
> > + reg = &labibb->lab;
> > + reg->desc.enable_mask = LAB_ENABLE_CTL_MASK;
> > + } else {
> > + reg = &labibb->ibb;
> > + reg->desc.enable_mask = IBB_ENABLE_CTL_MASK;
> > + }
>
> Write a switch statement so this is extensible.
I can change over to switch, though in the current set of downstream
code I've seen, it doesn't look that it would get extended. But I
guess there isn't any harm in moving over to switch. Will do.
Best,
Sumit.
^ permalink raw reply
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Felipe Balbi @ 2020-05-14 11:29 UTC (permalink / raw)
To: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Chandana Kishori Chiluveru, Sandeep Maheswaram
In-Reply-To: <1585718145-29537-3-git-send-email-sanm@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 367 bytes --]
Hi,
Sandeep Maheswaram <sanm@codeaurora.org> writes:
> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
> +{
> + struct device *dev = qcom->dev;
> + int ret;
> +
> + if (!device_is_bound(&qcom->dwc3->dev))
> + return -EPROBE_DEFER;
this breaks allmodconfig. I'm dropping this series from my queue for
this merge window.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* [PATCH 1/3] arm64: dts: qcom: msm8916: avoid using _ in node names
From: Stephan Gerhold @ 2020-05-14 11:27 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, devicetree, ~postmarketos/upstreaming,
Stephan Gerhold
Many nodes in the MSM8916 device trees use '_' in node names
(especially pinctrl), even though (seemingly) '-' is preferred now.
Make this more consistent by replacing '_' with '-' where possible.
Similar naming is used for pinctrl in newer device trees (e.g. sdm845.dtsi).
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
See also: https://lore.kernel.org/linux-arm-msm/20200512182959.GJ57962@builder.lan/
---
.../boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 14 +-
.../boot/dts/qcom/apq8016-sbc-soc-pins.dtsi | 12 +-
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 8 +-
.../boot/dts/qcom/msm8916-longcheer-l8150.dts | 2 +-
arch/arm64/boot/dts/qcom/msm8916-pins.dtsi | 181 +++++++++---------
.../qcom/msm8916-samsung-a2015-common.dtsi | 6 +-
arch/arm64/boot/dts/qcom/msm8916.dtsi | 10 +-
7 files changed, 116 insertions(+), 117 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
index aff218c1b7b6..41907938014c 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
@@ -4,7 +4,7 @@
&pm8916_gpios {
- usb_hub_reset_pm: usb_hub_reset_pm {
+ usb_hub_reset_pm: usb-hub-reset-pm {
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
@@ -13,7 +13,7 @@ pinconf {
};
};
- usb_hub_reset_pm_device: usb_hub_reset_pm_device {
+ usb_hub_reset_pm_device: usb-hub-reset-pm-device {
pinconf {
pins = "gpio3";
function = PMIC_GPIO_FUNC_NORMAL;
@@ -21,7 +21,7 @@ pinconf {
};
};
- usb_sw_sel_pm: usb_sw_sel_pm {
+ usb_sw_sel_pm: usb-sw-sel-pm {
pinconf {
pins = "gpio4";
function = PMIC_GPIO_FUNC_NORMAL;
@@ -31,7 +31,7 @@ pinconf {
};
};
- usb_sw_sel_pm_device: usb_sw_sel_pm_device {
+ usb_sw_sel_pm_device: usb-sw-sel-pm-device {
pinconf {
pins = "gpio4";
function = PMIC_GPIO_FUNC_NORMAL;
@@ -41,7 +41,7 @@ pinconf {
};
};
- pm8916_gpios_leds: pm8916_gpios_leds {
+ pm8916_gpios_leds: pm8916-gpios-leds {
pinconf {
pins = "gpio1", "gpio2";
function = PMIC_GPIO_FUNC_NORMAL;
@@ -55,7 +55,7 @@ &pm8916_mpps {
pinctrl-names = "default";
pinctrl-0 = <&ls_exp_gpio_f>;
- ls_exp_gpio_f: pm8916_mpp4 {
+ ls_exp_gpio_f: pm8916-mpp4 {
pinconf {
pins = "mpp4";
function = "digital";
@@ -64,7 +64,7 @@ pinconf {
};
};
- pm8916_mpps_leds: pm8916_mpps_leds {
+ pm8916_mpps_leds: pm8916-mpps-leds {
pinconf {
pins = "mpp2", "mpp3";
function = "digital";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
index 21d0822f1ca6..ac00beec102f 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
@@ -4,7 +4,7 @@
&msmgpio {
- msmgpio_leds: msmgpio_leds {
+ msmgpio_leds: msmgpio-leds {
pinconf {
pins = "gpio21", "gpio120";
function = "gpio";
@@ -26,7 +26,7 @@ pinconf {
};
};
- adv7533_int_active: adv533_int_active {
+ adv7533_int_active: adv533-int-active {
pinmux {
function = "gpio";
pins = "gpio31";
@@ -38,7 +38,7 @@ pinconf {
};
};
- adv7533_int_suspend: adv7533_int_suspend {
+ adv7533_int_suspend: adv7533-int-suspend {
pinmux {
function = "gpio";
pins = "gpio31";
@@ -50,7 +50,7 @@ pinconf {
};
};
- adv7533_switch_active: adv7533_switch_active {
+ adv7533_switch_active: adv7533-switch-active {
pinmux {
function = "gpio";
pins = "gpio32";
@@ -62,7 +62,7 @@ pinconf {
};
};
- adv7533_switch_suspend: adv7533_switch_suspend {
+ adv7533_switch_suspend: adv7533-switch-suspend {
pinmux {
function = "gpio";
pins = "gpio32";
@@ -74,7 +74,7 @@ pinconf {
};
};
- msm_key_volp_n_default: msm_key_volp_n_default {
+ msm_key_volp_n_default: msm-key-volp-n-default {
pinmux {
function = "gpio";
pins = "gpio107";
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 7377bf73390a..08c7538b9c9f 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -51,7 +51,7 @@ chosen {
stdout-path = "serial0";
};
- camera_vdddo_1v8: camera_vdddo_1v8 {
+ camera_vdddo_1v8: camera-vdddo-1v8 {
compatible = "regulator-fixed";
regulator-name = "camera_vdddo";
regulator-min-microvolt = <1800000>;
@@ -59,7 +59,7 @@ camera_vdddo_1v8: camera_vdddo_1v8 {
regulator-always-on;
};
- camera_vdda_2v8: camera_vdda_2v8 {
+ camera_vdda_2v8: camera-vdda-2v8 {
compatible = "regulator-fixed";
regulator-name = "camera_vdda";
regulator-min-microvolt = <2800000>;
@@ -67,7 +67,7 @@ camera_vdda_2v8: camera_vdda_2v8 {
regulator-always-on;
};
- camera_vddd_1v5: camera_vddd_1v5 {
+ camera_vddd_1v5: camera-vddd-1v5 {
compatible = "regulator-fixed";
regulator-name = "camera_vddd";
regulator-min-microvolt = <1500000>;
@@ -566,7 +566,7 @@ hdmi_con: endpoint {
};
};
- gpio_keys {
+ gpio-keys {
compatible = "gpio-keys";
#address-cells = <1>;
#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
index 691eb1a87bc9..d5230cb76eb1 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-longcheer-l8150.dts
@@ -109,7 +109,7 @@ volume-up {
};
&msmgpio {
- gpio_keys_default: gpio_keys_default {
+ gpio_keys_default: gpio-keys-default {
pinmux {
function = "gpio";
pins = "gpio107";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
index 31886860766a..e9c00367f7fd 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-pins.dtsi
@@ -5,7 +5,7 @@
&msmgpio {
- blsp1_uart1_default: blsp1_uart1_default {
+ blsp1_uart1_default: blsp1-uart1-default {
pinmux {
function = "blsp_uart1";
// TX, RX, CTS_N, RTS_N
@@ -20,7 +20,7 @@ pinconf {
};
};
- blsp1_uart1_sleep: blsp1_uart1_sleep {
+ blsp1_uart1_sleep: blsp1-uart1-sleep {
pinmux {
function = "gpio";
pins = "gpio0", "gpio1",
@@ -34,7 +34,7 @@ pinconf {
};
};
- blsp1_uart2_default: blsp1_uart2_default {
+ blsp1_uart2_default: blsp1-uart2-default {
pinmux {
function = "blsp_uart2";
pins = "gpio4", "gpio5";
@@ -46,7 +46,7 @@ pinconf {
};
};
- blsp1_uart2_sleep: blsp1_uart2_sleep {
+ blsp1_uart2_sleep: blsp1-uart2-sleep {
pinmux {
function = "gpio";
pins = "gpio4", "gpio5";
@@ -58,12 +58,12 @@ pinconf {
};
};
- spi1_default: spi1_default {
+ spi1_default: spi1-default {
pinmux {
function = "blsp_spi1";
pins = "gpio0", "gpio1", "gpio3";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio2";
};
@@ -72,7 +72,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio2";
drive-strength = <16>;
bias-disable;
@@ -80,7 +80,7 @@ pinconf_cs {
};
};
- spi1_sleep: spi1_sleep {
+ spi1_sleep: spi1-sleep {
pinmux {
function = "gpio";
pins = "gpio0", "gpio1", "gpio2", "gpio3";
@@ -92,12 +92,12 @@ pinconf {
};
};
- spi2_default: spi2_default {
+ spi2_default: spi2-default {
pinmux {
function = "blsp_spi2";
pins = "gpio4", "gpio5", "gpio7";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio6";
};
@@ -106,7 +106,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio6";
drive-strength = <16>;
bias-disable;
@@ -114,7 +114,7 @@ pinconf_cs {
};
};
- spi2_sleep: spi2_sleep {
+ spi2_sleep: spi2-sleep {
pinmux {
function = "gpio";
pins = "gpio4", "gpio5", "gpio6", "gpio7";
@@ -126,12 +126,12 @@ pinconf {
};
};
- spi3_default: spi3_default {
+ spi3_default: spi3-default {
pinmux {
function = "blsp_spi3";
pins = "gpio8", "gpio9", "gpio11";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio10";
};
@@ -140,7 +140,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio10";
drive-strength = <16>;
bias-disable;
@@ -148,7 +148,7 @@ pinconf_cs {
};
};
- spi3_sleep: spi3_sleep {
+ spi3_sleep: spi3-sleep {
pinmux {
function = "gpio";
pins = "gpio8", "gpio9", "gpio10", "gpio11";
@@ -160,12 +160,12 @@ pinconf {
};
};
- spi4_default: spi4_default {
+ spi4_default: spi4-default {
pinmux {
function = "blsp_spi4";
pins = "gpio12", "gpio13", "gpio15";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio14";
};
@@ -174,7 +174,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio14";
drive-strength = <16>;
bias-disable;
@@ -182,7 +182,7 @@ pinconf_cs {
};
};
- spi4_sleep: spi4_sleep {
+ spi4_sleep: spi4-sleep {
pinmux {
function = "gpio";
pins = "gpio12", "gpio13", "gpio14", "gpio15";
@@ -194,12 +194,12 @@ pinconf {
};
};
- spi5_default: spi5_default {
+ spi5_default: spi5-default {
pinmux {
function = "blsp_spi5";
pins = "gpio16", "gpio17", "gpio19";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio18";
};
@@ -208,7 +208,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio18";
drive-strength = <16>;
bias-disable;
@@ -216,7 +216,7 @@ pinconf_cs {
};
};
- spi5_sleep: spi5_sleep {
+ spi5_sleep: spi5-sleep {
pinmux {
function = "gpio";
pins = "gpio16", "gpio17", "gpio18", "gpio19";
@@ -228,12 +228,12 @@ pinconf {
};
};
- spi6_default: spi6_default {
+ spi6_default: spi6-default {
pinmux {
function = "blsp_spi6";
pins = "gpio20", "gpio21", "gpio23";
};
- pinmux_cs {
+ pinmux-cs {
function = "gpio";
pins = "gpio22";
};
@@ -242,7 +242,7 @@ pinconf {
drive-strength = <12>;
bias-disable;
};
- pinconf_cs {
+ pinconf-cs {
pins = "gpio22";
drive-strength = <16>;
bias-disable;
@@ -250,7 +250,7 @@ pinconf_cs {
};
};
- spi6_sleep: spi6_sleep {
+ spi6_sleep: spi6-sleep {
pinmux {
function = "gpio";
pins = "gpio20", "gpio21", "gpio22", "gpio23";
@@ -262,7 +262,7 @@ pinconf {
};
};
- i2c1_default: i2c1_default {
+ i2c1_default: i2c1-default {
pinmux {
function = "blsp_i2c1";
pins = "gpio2", "gpio3";
@@ -274,7 +274,7 @@ pinconf {
};
};
- i2c1_sleep: i2c1_sleep {
+ i2c1_sleep: i2c1-sleep {
pinmux {
function = "gpio";
pins = "gpio2", "gpio3";
@@ -286,7 +286,7 @@ pinconf {
};
};
- i2c2_default: i2c2_default {
+ i2c2_default: i2c2-default {
pinmux {
function = "blsp_i2c2";
pins = "gpio6", "gpio7";
@@ -298,7 +298,7 @@ pinconf {
};
};
- i2c2_sleep: i2c2_sleep {
+ i2c2_sleep: i2c2-sleep {
pinmux {
function = "gpio";
pins = "gpio6", "gpio7";
@@ -310,7 +310,7 @@ pinconf {
};
};
- i2c4_default: i2c4_default {
+ i2c4_default: i2c4-default {
pinmux {
function = "blsp_i2c4";
pins = "gpio14", "gpio15";
@@ -322,7 +322,7 @@ pinconf {
};
};
- i2c4_sleep: i2c4_sleep {
+ i2c4_sleep: i2c4-sleep {
pinmux {
function = "gpio";
pins = "gpio14", "gpio15";
@@ -334,7 +334,7 @@ pinconf {
};
};
- i2c5_default: i2c5_default {
+ i2c5_default: i2c5-default {
pinmux {
function = "blsp_i2c5";
pins = "gpio18", "gpio19";
@@ -346,7 +346,7 @@ pinconf {
};
};
- i2c5_sleep: i2c5_sleep {
+ i2c5_sleep: i2c5-sleep {
pinmux {
function = "gpio";
pins = "gpio18", "gpio19";
@@ -358,7 +358,7 @@ pinconf {
};
};
- i2c6_default: i2c6_default {
+ i2c6_default: i2c6-default {
pinmux {
function = "blsp_i2c6";
pins = "gpio22", "gpio23";
@@ -370,7 +370,7 @@ pinconf {
};
};
- i2c6_sleep: i2c6_sleep {
+ i2c6_sleep: i2c6-sleep {
pinmux {
function = "gpio";
pins = "gpio22", "gpio23";
@@ -382,8 +382,8 @@ pinconf {
};
};
- pmx_sdc1_clk {
- sdc1_clk_on: clk_on {
+ pmx-sdc1-clk {
+ sdc1_clk_on: clk-on {
pinmux {
pins = "sdc1_clk";
};
@@ -393,7 +393,7 @@ pinconf {
drive-strength = <16>;
};
};
- sdc1_clk_off: clk_off {
+ sdc1_clk_off: clk-off {
pinmux {
pins = "sdc1_clk";
};
@@ -405,8 +405,8 @@ pinconf {
};
};
- pmx_sdc1_cmd {
- sdc1_cmd_on: cmd_on {
+ pmx-sdc1-cmd {
+ sdc1_cmd_on: cmd-on {
pinmux {
pins = "sdc1_cmd";
};
@@ -416,7 +416,7 @@ pinconf {
drive-strength = <10>;
};
};
- sdc1_cmd_off: cmd_off {
+ sdc1_cmd_off: cmd-off {
pinmux {
pins = "sdc1_cmd";
};
@@ -428,8 +428,8 @@ pinconf {
};
};
- pmx_sdc1_data {
- sdc1_data_on: data_on {
+ pmx-sdc1-data {
+ sdc1_data_on: data-on {
pinmux {
pins = "sdc1_data";
};
@@ -439,7 +439,7 @@ pinconf {
drive-strength = <10>;
};
};
- sdc1_data_off: data_off {
+ sdc1_data_off: data-off {
pinmux {
pins = "sdc1_data";
};
@@ -451,8 +451,8 @@ pinconf {
};
};
- pmx_sdc2_clk {
- sdc2_clk_on: clk_on {
+ pmx-sdc2-clk {
+ sdc2_clk_on: clk-on {
pinmux {
pins = "sdc2_clk";
};
@@ -462,7 +462,7 @@ pinconf {
drive-strength = <16>;
};
};
- sdc2_clk_off: clk_off {
+ sdc2_clk_off: clk-off {
pinmux {
pins = "sdc2_clk";
};
@@ -474,8 +474,8 @@ pinconf {
};
};
- pmx_sdc2_cmd {
- sdc2_cmd_on: cmd_on {
+ pmx-sdc2-cmd {
+ sdc2_cmd_on: cmd-on {
pinmux {
pins = "sdc2_cmd";
};
@@ -485,7 +485,7 @@ pinconf {
drive-strength = <10>;
};
};
- sdc2_cmd_off: cmd_off {
+ sdc2_cmd_off: cmd-off {
pinmux {
pins = "sdc2_cmd";
};
@@ -497,8 +497,8 @@ pinconf {
};
};
- pmx_sdc2_data {
- sdc2_data_on: data_on {
+ pmx-sdc2-data {
+ sdc2_data_on: data-on {
pinmux {
pins = "sdc2_data";
};
@@ -508,7 +508,7 @@ pinconf {
drive-strength = <10>;
};
};
- sdc2_data_off: data_off {
+ sdc2_data_off: data-off {
pinmux {
pins = "sdc2_data";
};
@@ -520,8 +520,8 @@ pinconf {
};
};
- pmx_sdc2_cd_pin {
- sdc2_cd_on: cd_on {
+ pmx-sdc2-cd-pin {
+ sdc2_cd_on: cd-on {
pinmux {
function = "gpio";
pins = "gpio38";
@@ -532,7 +532,7 @@ pinconf {
bias-pull-up;
};
};
- sdc2_cd_off: cd_off {
+ sdc2_cd_off: cd-off {
pinmux {
function = "gpio";
pins = "gpio38";
@@ -546,7 +546,7 @@ pinconf {
};
cdc-pdm-lines {
- cdc_pdm_lines_act: pdm_lines_on {
+ cdc_pdm_lines_act: pdm-lines-on {
pinmux {
function = "cdc_pdm0";
pins = "gpio63", "gpio64", "gpio65", "gpio66",
@@ -559,7 +559,7 @@ pinconf {
bias-pull-none;
};
};
- cdc_pdm_lines_sus: pdm_lines_off {
+ cdc_pdm_lines_sus: pdm-lines-off {
pinmux {
function = "cdc_pdm0";
pins = "gpio63", "gpio64", "gpio65", "gpio66",
@@ -575,7 +575,7 @@ pinconf {
};
ext-pri-tlmm-lines {
- ext_pri_tlmm_lines_act: ext_pa_on {
+ ext_pri_tlmm_lines_act: ext-pa-on {
pinmux {
function = "pri_mi2s";
pins = "gpio113", "gpio114", "gpio115",
@@ -589,7 +589,7 @@ pinconf {
};
};
- ext_pri_tlmm_lines_sus: ext_pa_off {
+ ext_pri_tlmm_lines_sus: ext-pa-off {
pinmux {
function = "pri_mi2s";
pins = "gpio113", "gpio114", "gpio115",
@@ -605,7 +605,7 @@ pinconf {
};
ext-pri-ws-line {
- ext_pri_ws_act: ext_pa_on {
+ ext_pri_ws_act: ext-pa-on {
pinmux {
function = "pri_mi2s_ws";
pins = "gpio110";
@@ -617,7 +617,7 @@ pinconf {
};
};
- ext_pri_ws_sus: ext_pa_off {
+ ext_pri_ws_sus: ext-pa-off {
pinmux {
function = "pri_mi2s_ws";
pins = "gpio110";
@@ -631,7 +631,7 @@ pinconf {
};
ext-mclk-tlmm-lines {
- ext_mclk_tlmm_lines_act: mclk_lines_on {
+ ext_mclk_tlmm_lines_act: mclk-lines-on {
pinmux {
function = "pri_mi2s";
pins = "gpio116";
@@ -642,7 +642,7 @@ pinconf {
bias-pull-none;
};
};
- ext_mclk_tlmm_lines_sus: mclk_lines_off {
+ ext_mclk_tlmm_lines_sus: mclk-lines-off {
pinmux {
function = "pri_mi2s";
pins = "gpio116";
@@ -657,7 +657,7 @@ pinconf {
/* secondary Mi2S */
ext-sec-tlmm-lines {
- ext_sec_tlmm_lines_act: tlmm_lines_on {
+ ext_sec_tlmm_lines_act: tlmm-lines-on {
pinmux {
function = "sec_mi2s";
pins = "gpio112", "gpio117", "gpio118",
@@ -670,7 +670,7 @@ pinconf {
bias-pull-none;
};
};
- ext_sec_tlmm_lines_sus: tlmm_lines_off {
+ ext_sec_tlmm_lines_sus: tlmm-lines-off {
pinmux {
function = "sec_mi2s";
pins = "gpio112", "gpio117", "gpio118",
@@ -686,12 +686,12 @@ pinconf {
};
cdc-dmic-lines {
- cdc_dmic_lines_act: dmic_lines_on {
- pinmux_dmic0_clk {
+ cdc_dmic_lines_act: dmic-lines-on {
+ pinmux-dmic0-clk {
function = "dmic0_clk";
pins = "gpio0";
};
- pinmux_dmic0_data {
+ pinmux-dmic0-data {
function = "dmic0_data";
pins = "gpio1";
};
@@ -700,12 +700,12 @@ pinconf {
drive-strength = <8>;
};
};
- cdc_dmic_lines_sus: dmic_lines_off {
- pinmux_dmic0_clk {
+ cdc_dmic_lines_sus: dmic-lines-off {
+ pinmux-dmic0-clk {
function = "dmic0_clk";
pins = "gpio0";
};
- pinmux_dmic0_data {
+ pinmux-dmic0-data {
function = "dmic0_data";
pins = "gpio1";
};
@@ -722,7 +722,6 @@ pinmux {
pins = "gpio40", "gpio41", "gpio42", "gpio43", "gpio44";
function = "wcss_wlan";
};
-
pinconf {
pins = "gpio40", "gpio41", "gpio42", "gpio43", "gpio44";
drive-strength = <6>;
@@ -730,7 +729,7 @@ pinconf {
};
};
- cci0_default: cci0_default {
+ cci0_default: cci0-default {
pinmux {
function = "cci_i2c";
pins = "gpio29", "gpio30";
@@ -742,64 +741,64 @@ pinconf {
};
};
- camera_front_default: camera_front_default {
- pinmux_pwdn {
+ camera_front_default: camera-front-default {
+ pinmux-pwdn {
function = "gpio";
pins = "gpio33";
};
- pinconf_pwdn {
+ pinconf-pwdn {
pins = "gpio33";
drive-strength = <16>;
bias-disable;
};
- pinmux_rst {
+ pinmux-rst {
function = "gpio";
pins = "gpio28";
};
- pinconf_rst {
+ pinconf-rst {
pins = "gpio28";
drive-strength = <16>;
bias-disable;
};
- pinmux_mclk1 {
+ pinmux-mclk1 {
function = "cam_mclk1";
pins = "gpio27";
};
- pinconf_mclk1 {
+ pinconf-mclk1 {
pins = "gpio27";
drive-strength = <16>;
bias-disable;
};
};
- camera_rear_default: camera_rear_default {
- pinmux_pwdn {
+ camera_rear_default: camera-rear-default {
+ pinmux-pwdn {
function = "gpio";
pins = "gpio34";
};
- pinconf_pwdn {
+ pinconf-pwdn {
pins = "gpio34";
drive-strength = <16>;
bias-disable;
};
- pinmux_rst {
+ pinmux-rst {
function = "gpio";
pins = "gpio35";
};
- pinconf_rst {
+ pinconf-rst {
pins = "gpio35";
drive-strength = <16>;
bias-disable;
};
- pinmux_mclk0 {
+ pinmux-mclk0 {
function = "cam_mclk0";
pins = "gpio26";
};
- pinconf_mclk0 {
+ pinconf-mclk0 {
pins = "gpio26";
drive-strength = <16>;
bias-disable;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
index 8b5060f4fe0b..6f921e6054e1 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-a2015-common.dtsi
@@ -150,7 +150,7 @@ muic: sm5502@25 {
};
&msmgpio {
- gpio_keys_default: gpio_keys_default {
+ gpio_keys_default: gpio-keys-default {
pinmux {
function = "gpio";
pins = "gpio107", "gpio109";
@@ -162,7 +162,7 @@ pinconf {
};
};
- gpio_hall_sensor_default: gpio_hall_sensor_default {
+ gpio_hall_sensor_default: gpio-hall-sensor-default {
pinmux {
function = "gpio";
pins = "gpio52";
@@ -174,7 +174,7 @@ pinconf {
};
};
- muic_int_default: muic_int_default {
+ muic_int_default: muic-int-default {
pinmux {
function = "gpio";
pins = "gpio12";
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index cf9ce4a9c912..69129c0b20aa 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -340,7 +340,7 @@ modem_alert0: trip-point0 {
};
- cpu_opp_table: cpu_opp_table {
+ cpu_opp_table: cpu-opp-table {
compatible = "operating-points-v2";
opp-shared;
@@ -358,7 +358,7 @@ opp-998400000 {
};
};
- gpu_opp_table: opp_table {
+ gpu_opp_table: gpu-opp-table {
compatible = "operating-points-v2";
opp-400000000 {
@@ -378,13 +378,13 @@ timer {
};
clocks {
- xo_board: xo_board {
+ xo_board: xo-board {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <19200000>;
};
- sleep_clk: sleep_clk {
+ sleep_clk: sleep-clk {
compatible = "fixed-clock";
#clock-cells = <0>;
clock-frequency = <32768>;
@@ -1783,7 +1783,7 @@ rpm {
qcom,ipc = <&apcs 8 0>;
qcom,smd-edge = <15>;
- rpm_requests {
+ rpm-requests {
compatible = "qcom,rpm-msm8916";
qcom,smd-channels = "rpm_requests";
--
2.26.2
^ permalink raw reply related
* [PATCH 2/3] arm64: dts: qcom: msm8916: move gpu opp table to gpu node
From: Stephan Gerhold @ 2020-05-14 11:27 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, devicetree, ~postmarketos/upstreaming,
Stephan Gerhold
In-Reply-To: <20200514112754.148919-1-stephan@gerhold.net>
In msm8916.dtsi the GPU OPP table is defined under the root node,
whereas in msm8996.dtsi/sdm845.dtsi it is a subnode of the gpu device
(which makes it easier to find).
Move it to the gpu device node to make this consistent.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 69129c0b20aa..32bd140ac9fd 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -358,17 +358,6 @@ opp-998400000 {
};
};
- gpu_opp_table: gpu-opp-table {
- compatible = "operating-points-v2";
-
- opp-400000000 {
- opp-hz = /bits/ 64 <400000000>;
- };
- opp-19200000 {
- opp-hz = /bits/ 64 <19200000>;
- };
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 2 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>,
@@ -989,6 +978,17 @@ gpu@1c00000 {
power-domains = <&gcc OXILI_GDSC>;
operating-points-v2 = <&gpu_opp_table>;
iommus = <&gpu_iommu 1>, <&gpu_iommu 2>;
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ };
+ opp-19200000 {
+ opp-hz = /bits/ 64 <19200000>;
+ };
+ };
};
mdss: mdss@1a00000 {
--
2.26.2
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: qcom: apq8016-sbc: merge -pins.dtsi into main .dtsi
From: Stephan Gerhold @ 2020-05-14 11:27 UTC (permalink / raw)
To: Andy Gross, Bjorn Andersson
Cc: linux-arm-msm, devicetree, ~postmarketos/upstreaming,
Stephan Gerhold
In-Reply-To: <20200514112754.148919-1-stephan@gerhold.net>
apq8016-sbc.dtsi is the only remaining device which takes up
4 files since it has its pinctrl split into separate files.
Actually this does not really make the device tree easier to read
(just harder to find nodes). For db820c the files were merged in
commit 88264f1f6bf5 ("arm64: dts: qcom: db820c: Remove pin specific files").
Do the same for apq8016-sbc (db410c) and move the pinctrl definitions
into apq8016-sbc.dtsi.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
.../boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi | 74 --------
.../boot/dts/qcom/apq8016-sbc-soc-pins.dtsi | 89 ----------
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi | 158 +++++++++++++++++-
3 files changed, 156 insertions(+), 165 deletions(-)
delete mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
delete mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
deleted file mode 100644
index 41907938014c..000000000000
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-pmic-pins.dtsi
+++ /dev/null
@@ -1,74 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
-#include <dt-bindings/pinctrl/qcom,pmic-mpp.h>
-
-&pm8916_gpios {
-
- usb_hub_reset_pm: usb-hub-reset-pm {
- pinconf {
- pins = "gpio3";
- function = PMIC_GPIO_FUNC_NORMAL;
- input-disable;
- output-high;
- };
- };
-
- usb_hub_reset_pm_device: usb-hub-reset-pm-device {
- pinconf {
- pins = "gpio3";
- function = PMIC_GPIO_FUNC_NORMAL;
- output-low;
- };
- };
-
- usb_sw_sel_pm: usb-sw-sel-pm {
- pinconf {
- pins = "gpio4";
- function = PMIC_GPIO_FUNC_NORMAL;
- power-source = <PM8916_GPIO_VPH>;
- input-disable;
- output-high;
- };
- };
-
- usb_sw_sel_pm_device: usb-sw-sel-pm-device {
- pinconf {
- pins = "gpio4";
- function = PMIC_GPIO_FUNC_NORMAL;
- power-source = <PM8916_GPIO_VPH>;
- input-disable;
- output-low;
- };
- };
-
- pm8916_gpios_leds: pm8916-gpios-leds {
- pinconf {
- pins = "gpio1", "gpio2";
- function = PMIC_GPIO_FUNC_NORMAL;
- output-low;
- };
- };
-};
-
-&pm8916_mpps {
-
- pinctrl-names = "default";
- pinctrl-0 = <&ls_exp_gpio_f>;
-
- ls_exp_gpio_f: pm8916-mpp4 {
- pinconf {
- pins = "mpp4";
- function = "digital";
- output-low;
- power-source = <PM8916_MPP_L5>; // 1.8V
- };
- };
-
- pm8916_mpps_leds: pm8916-mpps-leds {
- pinconf {
- pins = "mpp2", "mpp3";
- function = "digital";
- output-low;
- };
- };
-};
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
deleted file mode 100644
index ac00beec102f..000000000000
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-soc-pins.dtsi
+++ /dev/null
@@ -1,89 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-#include <dt-bindings/gpio/gpio.h>
-
-&msmgpio {
-
- msmgpio_leds: msmgpio-leds {
- pinconf {
- pins = "gpio21", "gpio120";
- function = "gpio";
- output-low;
- };
- };
-
- usb_id_default: usb-id-default {
- pinmux {
- function = "gpio";
- pins = "gpio121";
- };
-
- pinconf {
- pins = "gpio121";
- drive-strength = <8>;
- input-enable;
- bias-pull-up;
- };
- };
-
- adv7533_int_active: adv533-int-active {
- pinmux {
- function = "gpio";
- pins = "gpio31";
- };
- pinconf {
- pins = "gpio31";
- drive-strength = <16>;
- bias-disable;
- };
- };
-
- adv7533_int_suspend: adv7533-int-suspend {
- pinmux {
- function = "gpio";
- pins = "gpio31";
- };
- pinconf {
- pins = "gpio31";
- drive-strength = <2>;
- bias-disable;
- };
- };
-
- adv7533_switch_active: adv7533-switch-active {
- pinmux {
- function = "gpio";
- pins = "gpio32";
- };
- pinconf {
- pins = "gpio32";
- drive-strength = <16>;
- bias-disable;
- };
- };
-
- adv7533_switch_suspend: adv7533-switch-suspend {
- pinmux {
- function = "gpio";
- pins = "gpio32";
- };
- pinconf {
- pins = "gpio32";
- drive-strength = <2>;
- bias-disable;
- };
- };
-
- msm_key_volp_n_default: msm-key-volp-n-default {
- pinmux {
- function = "gpio";
- pins = "gpio107";
- };
- pinconf {
- pins = "gpio107";
- drive-strength = <8>;
- input-enable;
- bias-pull-up;
- };
- };
-};
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
index 08c7538b9c9f..8a4b790aa7ff 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
@@ -5,10 +5,10 @@
#include "msm8916.dtsi"
#include "pm8916.dtsi"
-#include "apq8016-sbc-soc-pins.dtsi"
-#include "apq8016-sbc-pmic-pins.dtsi"
#include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/input/input.h>
+#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
+#include <dt-bindings/pinctrl/qcom,pmic-mpp.h>
#include <dt-bindings/sound/apq8016-lpass.h>
/*
@@ -777,3 +777,157 @@ l18 {
regulator-max-microvolt = <3337000>;
};
};
+
+&msmgpio {
+ msmgpio_leds: msmgpio-leds {
+ pinconf {
+ pins = "gpio21", "gpio120";
+ function = "gpio";
+ output-low;
+ };
+ };
+
+ usb_id_default: usb-id-default {
+ pinmux {
+ function = "gpio";
+ pins = "gpio121";
+ };
+
+ pinconf {
+ pins = "gpio121";
+ drive-strength = <8>;
+ input-enable;
+ bias-pull-up;
+ };
+ };
+
+ adv7533_int_active: adv533-int-active {
+ pinmux {
+ function = "gpio";
+ pins = "gpio31";
+ };
+ pinconf {
+ pins = "gpio31";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
+ adv7533_int_suspend: adv7533-int-suspend {
+ pinmux {
+ function = "gpio";
+ pins = "gpio31";
+ };
+ pinconf {
+ pins = "gpio31";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
+ adv7533_switch_active: adv7533-switch-active {
+ pinmux {
+ function = "gpio";
+ pins = "gpio32";
+ };
+ pinconf {
+ pins = "gpio32";
+ drive-strength = <16>;
+ bias-disable;
+ };
+ };
+
+ adv7533_switch_suspend: adv7533-switch-suspend {
+ pinmux {
+ function = "gpio";
+ pins = "gpio32";
+ };
+ pinconf {
+ pins = "gpio32";
+ drive-strength = <2>;
+ bias-disable;
+ };
+ };
+
+ msm_key_volp_n_default: msm-key-volp-n-default {
+ pinmux {
+ function = "gpio";
+ pins = "gpio107";
+ };
+ pinconf {
+ pins = "gpio107";
+ drive-strength = <8>;
+ input-enable;
+ bias-pull-up;
+ };
+ };
+};
+
+&pm8916_gpios {
+ usb_hub_reset_pm: usb-hub-reset-pm {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_hub_reset_pm_device: usb-hub-reset-pm-device {
+ pinconf {
+ pins = "gpio3";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ output-low;
+ };
+ };
+
+ usb_sw_sel_pm: usb-sw-sel-pm {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = <PM8916_GPIO_VPH>;
+ input-disable;
+ output-high;
+ };
+ };
+
+ usb_sw_sel_pm_device: usb-sw-sel-pm-device {
+ pinconf {
+ pins = "gpio4";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ power-source = <PM8916_GPIO_VPH>;
+ input-disable;
+ output-low;
+ };
+ };
+
+ pm8916_gpios_leds: pm8916-gpios-leds {
+ pinconf {
+ pins = "gpio1", "gpio2";
+ function = PMIC_GPIO_FUNC_NORMAL;
+ output-low;
+ };
+ };
+};
+
+&pm8916_mpps {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ls_exp_gpio_f>;
+
+ ls_exp_gpio_f: pm8916-mpp4 {
+ pinconf {
+ pins = "mpp4";
+ function = "digital";
+ output-low;
+ power-source = <PM8916_MPP_L5>; // 1.8V
+ };
+ };
+
+ pm8916_mpps_leds: pm8916-mpps-leds {
+ pinconf {
+ pins = "mpp2", "mpp3";
+ function = "digital";
+ output-low;
+ };
+ };
+};
--
2.26.2
^ permalink raw reply related
* [PATCH] dt-bindings: auxdisplay: hd44780: Convert to json-schema
From: Geert Uytterhoeven @ 2020-05-14 11:30 UTC (permalink / raw)
To: Miguel Ojeda Sandonis, Rob Herring
Cc: devicetree, linux-kernel, Geert Uytterhoeven
Convert the Hitachi HD44780 Character LCD Controller Device Tree binding
documentation to json-schema.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
.../bindings/auxdisplay/hit,hd44780.txt | 45 ---------
.../bindings/auxdisplay/hit,hd44780.yaml | 96 +++++++++++++++++++
2 files changed, 96 insertions(+), 45 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
create mode 100644 Documentation/devicetree/bindings/auxdisplay/hit,hd44780.yaml
diff --git a/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt b/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
deleted file mode 100644
index 2aa24b8899236882..0000000000000000
--- a/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.txt
+++ /dev/null
@@ -1,45 +0,0 @@
-DT bindings for the Hitachi HD44780 Character LCD Controller
-
-The Hitachi HD44780 Character LCD Controller is commonly used on character LCDs
-that can display one or more lines of text. It exposes an M6800 bus interface,
-which can be used in either 4-bit or 8-bit mode.
-
-Required properties:
- - compatible: Must contain "hit,hd44780",
- - data-gpios: Must contain an array of either 4 or 8 GPIO specifiers,
- referring to the GPIO pins connected to the data signal lines DB0-DB7
- (8-bit mode) or DB4-DB7 (4-bit mode) of the LCD Controller's bus interface,
- - enable-gpios: Must contain a GPIO specifier, referring to the GPIO pin
- connected to the "E" (Enable) signal line of the LCD Controller's bus
- interface,
- - rs-gpios: Must contain a GPIO specifier, referring to the GPIO pin
- connected to the "RS" (Register Select) signal line of the LCD Controller's
- bus interface,
- - display-height-chars: Height of the display, in character cells,
- - display-width-chars: Width of the display, in character cells.
-
-Optional properties:
- - rw-gpios: Must contain a GPIO specifier, referring to the GPIO pin
- connected to the "RW" (Read/Write) signal line of the LCD Controller's bus
- interface,
- - backlight-gpios: Must contain a GPIO specifier, referring to the GPIO pin
- used for enabling the LCD's backlight,
- - internal-buffer-width: Internal buffer width (default is 40 for displays
- with 1 or 2 lines, and display-width-chars for displays with more than 2
- lines).
-
-Example:
-
- auxdisplay {
- compatible = "hit,hd44780";
-
- data-gpios = <&hc595 0 GPIO_ACTIVE_HIGH>,
- <&hc595 1 GPIO_ACTIVE_HIGH>,
- <&hc595 2 GPIO_ACTIVE_HIGH>,
- <&hc595 3 GPIO_ACTIVE_HIGH>;
- enable-gpios = <&hc595 4 GPIO_ACTIVE_HIGH>;
- rs-gpios = <&hc595 5 GPIO_ACTIVE_HIGH>;
-
- display-height-chars = <2>;
- display-width-chars = <16>;
- };
diff --git a/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.yaml b/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.yaml
new file mode 100644
index 0000000000000000..9222b06e93a042e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/auxdisplay/hit,hd44780.yaml
@@ -0,0 +1,96 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/auxdisplay/hit,hd44780.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hitachi HD44780 Character LCD Controller
+
+maintainers:
+ - Geert Uytterhoeven <geert@linux-m68k.org>
+
+description:
+ The Hitachi HD44780 Character LCD Controller is commonly used on character
+ LCDs that can display one or more lines of text. It exposes an M6800 bus
+ interface, which can be used in either 4-bit or 8-bit mode.
+
+properties:
+ compatible:
+ const: hit,hd44780
+
+ data-gpios:
+ description:
+ GPIO pins connected to the data signal lines DB0-DB7 (8-bit mode) or
+ DB4-DB7 (4-bit mode) of the LCD Controller's bus interface.
+ oneOf:
+ - maxItems: 4
+ - maxItems: 8
+
+ enable-gpios:
+ description:
+ GPIO pin connected to the "E" (Enable) signal line of the LCD
+ Controller's bus interface.
+ maxItems: 1
+
+ rs-gpios:
+ description:
+ GPIO pin connected to the "RS" (Register Select) signal line of the LCD
+ Controller's bus interface.
+ maxItems: 1
+
+ rw-gpios:
+ description:
+ GPIO pin connected to the "RW" (Read/Write) signal line of the LCD
+ Controller's bus interface.
+ maxItems: 1
+
+ backlight-gpios:
+ description: GPIO pin used for enabling the LCD's backlight.
+ maxItems: 1
+
+ display-height-chars:
+ description: Height of the display, in character cells,
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 4
+
+ display-width-chars:
+ description: Width of the display, in character cells.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 64
+
+ internal-buffer-width:
+ description:
+ Internal buffer width (default is 40 for displays with 1 or 2 lines, and
+ display-width-chars for displays with more than 2 lines).
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1
+ maximum: 64
+
+required:
+ - compatible
+ - data-gpios
+ - enable-gpios
+ - rs-gpios
+ - display-height-chars
+ - display-width-chars
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ auxdisplay {
+ compatible = "hit,hd44780";
+
+ data-gpios = <&hc595 0 GPIO_ACTIVE_HIGH>,
+ <&hc595 1 GPIO_ACTIVE_HIGH>,
+ <&hc595 2 GPIO_ACTIVE_HIGH>,
+ <&hc595 3 GPIO_ACTIVE_HIGH>;
+ enable-gpios = <&hc595 4 GPIO_ACTIVE_HIGH>;
+ rs-gpios = <&hc595 5 GPIO_ACTIVE_HIGH>;
+
+ display-height-chars = <2>;
+ display-width-chars = <16>;
+ };
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v7 2/4] usb: dwc3: qcom: Add interconnect support in dwc3 driver
From: Felipe Balbi @ 2020-05-14 11:30 UTC (permalink / raw)
To: Sandeep Maheswaram, Andy Gross, Bjorn Andersson,
Greg Kroah-Hartman, Rob Herring, Mark Rutland, Stephen Boyd,
Doug Anderson, Matthias Kaehlcke
Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Manu Gautam,
Chandana Kishori Chiluveru, Sandeep Maheswaram
In-Reply-To: <878shu4uwk.fsf@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 458 bytes --]
Felipe Balbi <balbi@kernel.org> writes:
> Hi,
>
> Sandeep Maheswaram <sanm@codeaurora.org> writes:
>> +static int dwc3_qcom_interconnect_init(struct dwc3_qcom *qcom)
>> +{
>> + struct device *dev = qcom->dev;
>> + int ret;
>> +
>> + if (!device_is_bound(&qcom->dwc3->dev))
>> + return -EPROBE_DEFER;
>
> this breaks allmodconfig. I'm dropping this series from my queue for
> this merge window.
Sorry, I meant this patch ;-)
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ 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