Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4 2/6] mfd: bd71837: Devicetree bindings for ROHM BD71837 PMIC
From: Matti Vaittinen @ 2018-05-31  7:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: Matti Vaittinen, mturquette, sboyd, mark.rutland, lee.jones,
	lgirdwood, broonie, mazziesaccount, linux-clk, devicetree,
	linux-kernel, mikko.mutanen, heikki.haikola
In-Reply-To: <20180531030129.GA16122@rob-hp-laptop>

Hello Rob,

Thanks for the review!

On Wed, May 30, 2018 at 10:01:29PM -0500, Rob Herring wrote:
> On Wed, May 30, 2018 at 11:42:03AM +0300, Matti Vaittinen wrote:
> > Document devicetree bindings for ROHM BD71837 PMIC MFD.
> > + - interrupts		: The interrupt line the device is connected to.
> > + - interrupt-controller	: Marks the device node as an interrupt controller.
> 
> What sub blocks have interrupts?

The PMIC can generate interrupts from events which cause it to reset.
Eg, irq from watchdog line change, power button pushes, reset request
via register interface etc. I don't know any generic handling for these
interrupts. In "normal" use-case this PMIC is powering the processor
where driver is running and I do not see reasonable handling because
power-reset is going to follow the irq.

This IRQ might be relevant if use for PMIC is such that it is not
powering the processor where the driver is runninng. Then the controlling
processor can get the notification that chips powered by PMIC are
resetting. But handling for this must be use-case specific, right? So in
short - none of the current sub-devices use the IRQs - they are there
for specific use-cases which some boards may implement. Any suggestions
how to document the available interrupts? (power button line, sw reset
etc). My current assumption has been that one who is interested in using
these irqs should really also see the data-sheet for IRQs. But I admit
that documenting available interrupts here would be helpful. I will just
cook up some explanation and send it as a patch if no suggestions on how
to document those.

Patches 3/6 and 6/6 from the series were already applied to Mark's tree.
So how should I send further patches? Should I still send the whole series
(including already applied patches 3/6 and 6/6) or only the ones I change?

> > +Example:
> > +
> > +	pmic: bd71837@4b {
> 
> Node names should be generic ideally. So "pmic@4b"

I'll change that.

> > +		clk: bd71837-32k-out {
> 
> clock-controller {

And I'll change that too.

> 
> > +			compatible = "rohm,bd71837-clk";
> > +			#clock-cells = <0>;
> > +			clock-frequency = <32768>;
> 
> Can this be anything else?

Not so that I know. Frequency is fixed. Is there a problem with this?


Br,
	Matti Vaittinen

^ permalink raw reply

* Re: [PATCH v1 2/2] hwmon: npcm-pwm: add NPCM7xx PWM driver
From: kbuild test robot @ 2018-05-31  7:16 UTC (permalink / raw)
  Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, linux, avifishman70,
	yuenn, brendanhiggins, venture, joel, devicetree, linux-kernel,
	linux-hwmon, openbmc, Tomer Maimon
In-Reply-To: <1527588141-18639-3-git-send-email-tmaimon77@gmail.com>

Hi Tomer,

I love your patch! Perhaps something to improve:

[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.17-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tomer-Maimon/dt-binding-hwmon-Add-NPCM7xx-PWM-documentation/20180531-034040
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: x86_64-allmodconfig
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        make ARCH=x86_64  allmodconfig
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):


vim +277 drivers/hwmon/npcm7xx-pwm.c

   250	
   251	static int npcm7xx_pwm_probe(struct platform_device *pdev)
   252	{
   253		struct device *dev = &pdev->dev;
   254		struct npcm7xx_pwm_data *data;
   255		struct resource res[NPCM7XX_PWM_MAX_MODULES];
   256		struct device *hwmon;
   257		struct clk *clk;
   258		int m, ch, res_cnt, ret;
   259		u32 Prescale_val, output_freq;
   260	
   261		data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
   262		if (!data)
   263			return -ENOMEM;
   264	
   265		for (res_cnt = 0; res_cnt < NPCM7XX_PWM_MAX_MODULES  ; res_cnt++) {
   266			ret = of_address_to_resource(dev->of_node, res_cnt,
   267						     &res[res_cnt]);
   268			if (ret) {
   269				pr_err("PWM of_address_to_resource fail ret %d\n",
   270				       ret);
   271				return -EINVAL;
   272			}
   273	
   274			data->pwm_base[res_cnt] =
   275				devm_ioremap_resource(dev, &(res[res_cnt]));
 > 276			pr_debug("pwm%d base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
 > 277				 res_cnt, (u32)data->pwm_base[res_cnt],
   278				 res[res_cnt].start, resource_size(&(res[res_cnt])));
   279	
   280			if (!data->pwm_base[res_cnt]) {
   281				pr_err("pwm probe failed: can't read pwm base address for resource %d.\n",
   282				       res_cnt);
   283				return -ENOMEM;
   284			}
   285	
   286			mutex_init(&data->npcm7xx_pwm_lock[res_cnt]);
   287		}
   288	
   289		clk = devm_clk_get(dev, NULL);
   290		if (IS_ERR(clk))
   291			return -ENODEV;
   292	
   293		data->clk_freq = clk_get_rate(clk);
   294	
   295		/* Adjust NPCM7xx PWMs output frequency to ~25Khz */
   296		output_freq = data->clk_freq / PWN_CNT_DEFAULT;
   297		Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
   298	
   299		/* If Prescale_val = 0, then the prescale output clock is stopped */
   300		if (Prescale_val < MIN_PRESCALE1)
   301			Prescale_val = MIN_PRESCALE1;
   302		/*
   303		 * Prescale_val need to decrement in one because in the PWM Prescale
   304		 * register the Prescale value increment by one
   305		 */
   306		Prescale_val--;
   307	
   308		/* Setting PWM Prescale Register value register to both modules */
   309		Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
   310	
   311		for (m = 0; m < NPCM7XX_PWM_MAX_MODULES  ; m++) {
   312			iowrite32(Prescale_val,
   313				  data->pwm_base[m] + NPCM7XX_PWM_REG_PR);
   314			iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
   315				  data->pwm_base[m] + NPCM7XX_PWM_REG_CSR);
   316			iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
   317				  data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
   318	
   319			for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM; ch++) {
   320				iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
   321					  data->pwm_base[m] + NPCM7XX_PWM_REG_CNRx(ch));
   322				iowrite32(NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM,
   323					  data->pwm_base[m] + NPCM7XX_PWM_REG_CMRx(ch));
   324			}
   325	
   326			iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT |
   327				  NPCM7XX_PWM_CTRL_EN_DEFALUT,
   328				  data->pwm_base[m] + NPCM7XX_PWM_REG_CR);
   329		}
   330	
   331		hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm", data,
   332							     &npcm7xx_chip_info, NULL);
   333	
   334		if (IS_ERR(hwmon)) {
   335			pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
   336			return PTR_ERR(hwmon);
   337		}
   338	
   339		pr_info("NPCM7XX PWM Driver probed, PWM output Freq %dHz\n",
   340			output_freq / ((Prescale_val & 0xf) + 1));
   341	
   342		return 0;
   343	}
   344	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* Re: [PATCH v2 08/10] drm/bridge: tc358764: Add DSI to LVDS bridge driver
From: Archit Taneja @ 2018-05-31  6:51 UTC (permalink / raw)
  To: Maciej Purski, linux-kernel, linux-arm-kernel, linux-samsung-soc,
	devicetree, dri-devel
  Cc: Mark Rutland, Laurent Pinchart, Bartlomiej Zolnierkiewicz,
	David Airlie, Seung-Woo Kim, Krzysztof Kozlowski, Kyungmin Park,
	Rob Herring, Thierry Reding, Kukjin Kim, Marek Szyprowski
In-Reply-To: <1527682561-1386-9-git-send-email-m.purski@samsung.com>

Hi,

On Wednesday 30 May 2018 05:45 PM, Maciej Purski wrote:
> From: Andrzej Hajda <a.hajda@samsung.com>
> 
> Add a drm_bridge driver for the Toshiba TC358764 DSI to LVDS bridge.
> 
> Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Maciej Purski <m.purski@samsung.com>
> ---
>   drivers/gpu/drm/bridge/Kconfig    |   9 +
>   drivers/gpu/drm/bridge/Makefile   |   1 +
>   drivers/gpu/drm/bridge/tc358764.c | 547 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 557 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/tc358764.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index fa2c799..9bd3eb8 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -110,6 +110,15 @@ config DRM_THINE_THC63LVD1024
>   	---help---
>   	  Thine THC63LVD1024 LVDS/parallel converter driver.
>   
> +config DRM_TOSHIBA_TC358764
> +	tristate "TC358764 DSI/LVDS bridge"
> +	depends on DRM && DRM_PANEL
> +	depends on OF
> +	select DRM_MIPI_DSI
> +	select VIDEOMODE_HELPERS

I don't see videomode usage in the driver, can we drop this if it isn't
used?

> +	help
> +	  Toshiba TC358764 DSI/LVDS bridge driver
> +
>   config DRM_TOSHIBA_TC358767
>   	tristate "Toshiba TC358767 eDP bridge"
>   	depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 35f88d4..bf7c0ce 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o
>   obj-$(CONFIG_DRM_SII902X) += sii902x.o
>   obj-$(CONFIG_DRM_SII9234) += sii9234.o
>   obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o
> +obj-$(CONFIG_DRM_TOSHIBA_TC358764) += tc358764.o
>   obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
>   obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> diff --git a/drivers/gpu/drm/bridge/tc358764.c b/drivers/gpu/drm/bridge/tc358764.c
> new file mode 100644
> index 0000000..3109eba
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/tc358764.c
> @@ -0,0 +1,547 @@

We'd need a SPDX license here?
> +/*
> + * Copyright (C) 2018 Samsung Electronics Co., Ltd
> + *
> + * Authors:
> + *	Andrzej Hajda <a.hajda@samsung.com>
> + *	Maciej Purski <m.purski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.
> + *
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_mipi_dsi.h>
> +#include <drm/drm_panel.h>
> +
> +#include <drm/drm_crtc.h>
> +#include <drm/drm_crtc_helper.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <video/mipi_display.h>
> +#include <video/of_videomode.h>
> +#include <video/videomode.h>
> +
> +#define FLD_MASK(start, end)    (((1 << ((start) - (end) + 1)) - 1) << (end))
> +#define FLD_VAL(val, start, end) (((val) << (end)) & FLD_MASK(start, end))
> +
> +/* PPI layer registers */
> +#define PPI_STARTPPI		0x0104 /* START control bit */
> +#define PPI_LPTXTIMECNT		0x0114 /* LPTX timing signal */
> +#define PPI_LANEENABLE		0x0134 /* Enables each lane */
> +#define PPI_TX_RX_TA		0x013C /* BTA timing parameters */
> +#define PPI_D0S_CLRSIPOCOUNT	0x0164 /* Assertion timer for Lane 0 */
> +#define PPI_D1S_CLRSIPOCOUNT	0x0168 /* Assertion timer for Lane 1 */
> +#define PPI_D2S_CLRSIPOCOUNT	0x016C /* Assertion timer for Lane 2 */
> +#define PPI_D3S_CLRSIPOCOUNT	0x0170 /* Assertion timer for Lane 3 */
> +#define PPI_START_FUNCTION	1
> +
> +/* DSI layer registers */
> +#define DSI_STARTDSI		0x0204 /* START control bit of DSI-TX */
> +#define DSI_LANEENABLE		0x0210 /* Enables each lane */
> +#define DSI_RX_START		1
> +
> +/* Video path registers */
> +#define VP_CTRL			0x0450 /* Video Path Control */
> +#define VP_CTRL_MSF(v)		FLD_VAL(v, 0, 0) /* Magic square in RGB666 */
> +#define VP_CTRL_VTGEN(v)	FLD_VAL(v, 4, 4) /* Use chip clock for timing */
> +#define VP_CTRL_EVTMODE(v)	FLD_VAL(v, 5, 5) /* Event mode */
> +#define VP_CTRL_RGB888(v)	FLD_VAL(v, 8, 8) /* RGB888 mode */
> +#define VP_CTRL_VSDELAY(v)	FLD_VAL(v, 31, 20) /* VSYNC delay */
> +#define VP_CTRL_HSPOL		BIT(17) /* Polarity of HSYNC signal */
> +#define VP_CTRL_DEPOL		BIT(18) /* Polarity of DE signal */
> +#define VP_CTRL_VSPOL		BIT(19) /* Polarity of VSYNC signal */
> +#define VP_HTIM1		0x0454 /* Horizontal Timing Control 1 */
> +#define VP_HTIM1_HBP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM1_HSYNC(v)	FLD_VAL(v, 8, 0)
> +#define VP_HTIM2		0x0458 /* Horizontal Timing Control 2 */
> +#define VP_HTIM2_HFP(v)		FLD_VAL(v, 24, 16)
> +#define VP_HTIM2_HACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VTIM1		0x045C /* Vertical Timing Control 1 */
> +#define VP_VTIM1_VBP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM1_VSYNC(v)	FLD_VAL(v, 7, 0)
> +#define VP_VTIM2		0x0460 /* Vertical Timing Control 2 */
> +#define VP_VTIM2_VFP(v)		FLD_VAL(v, 23, 16)
> +#define VP_VTIM2_VACT(v)	FLD_VAL(v, 10, 0)
> +#define VP_VFUEN		0x0464 /* Video Frame Timing Update Enable */
> +
> +/* LVDS registers */
> +#define LV_MX0003		0x0480 /* Mux input bit 0 to 3 */
> +#define LV_MX0407		0x0484 /* Mux input bit 4 to 7 */
> +#define LV_MX0811		0x0488 /* Mux input bit 8 to 11 */
> +#define LV_MX1215		0x048C /* Mux input bit 12 to 15 */
> +#define LV_MX1619		0x0490 /* Mux input bit 16 to 19 */
> +#define LV_MX2023		0x0494 /* Mux input bit 20 to 23 */
> +#define LV_MX2427		0x0498 /* Mux input bit 24 to 27 */
> +#define LV_MX(b0, b1, b2, b3)	(FLD_VAL(b0, 4, 0) | FLD_VAL(b1, 12, 8) | \
> +				FLD_VAL(b2, 20, 16) | FLD_VAL(b3, 28, 24))
> +
> +/* Input bit numbers used in mux registers */
> +enum {
> +	LVI_R0,
> +	LVI_R1,
> +	LVI_R2,
> +	LVI_R3,
> +	LVI_R4,
> +	LVI_R5,
> +	LVI_R6,
> +	LVI_R7,
> +	LVI_G0,
> +	LVI_G1,
> +	LVI_G2,
> +	LVI_G3,
> +	LVI_G4,
> +	LVI_G5,
> +	LVI_G6,
> +	LVI_G7,
> +	LVI_B0,
> +	LVI_B1,
> +	LVI_B2,
> +	LVI_B3,
> +	LVI_B4,
> +	LVI_B5,
> +	LVI_B6,
> +	LVI_B7,
> +	LVI_HS,
> +	LVI_VS,
> +	LVI_DE,
> +	LVI_L0
> +};
> +
> +#define LV_CFG			0x049C /* LVDS Configuration */
> +#define LV_PHY0			0x04A0 /* LVDS PHY 0 */
> +#define LV_PHY0_RST(v)		FLD_VAL(v, 22, 22) /* PHY reset */
> +#define LV_PHY0_IS(v)		FLD_VAL(v, 15, 14)
> +#define LV_PHY0_ND(v)		FLD_VAL(v, 4, 0) /* Frequency range select */
> +#define LV_PHY0_PRBS_ON(v)	FLD_VAL(v, 20, 16) /* Clock/Data Flag pins */
> +
> +/* System registers */
> +#define SYS_RST			0x0504 /* System Reset */
> +#define SYS_ID			0x0580 /* System ID */
> +
> +#define SYS_RST_I2CS		BIT(0) /* Reset I2C-Slave controller */
> +#define SYS_RST_I2CM		BIT(1) /* Reset I2C-Master controller */
> +#define SYS_RST_LCD		BIT(2) /* Reset LCD controller */
> +#define SYS_RST_BM		BIT(3) /* Reset Bus Management controller */
> +#define SYS_RST_DSIRX		BIT(4) /* Reset DSI-RX and App controller */
> +#define SYS_RST_REG		BIT(5) /* Reset Register module */
> +
> +#define LPX_PERIOD		2
> +#define TTA_SURE		3
> +#define TTA_GET			0x20000
> +
> +/* Lane enable PPI and DSI register bits */
> +#define LANEENABLE_CLEN		BIT(0)
> +#define LANEENABLE_L0EN		BIT(1)
> +#define LANEENABLE_L1EN		BIT(2)
> +#define LANEENABLE_L2EN		BIT(3)
> +#define LANEENABLE_L3EN		BIT(4)
> +
> +/* LVCFG fields */
> +#define LV_CFG_LVEN		BIT(0)
> +#define LV_CFG_LVDLINK		BIT(1)
> +#define LV_CFG_CLKPOL1		BIT(2)
> +#define LV_CFG_CLKPOL2		BIT(3)
> +
> +static const char * const tc358764_supplies[] = {
> +	"vddc", "vddio", "vddmipi", "vddlvds133", "vddlvds112"
> +};
> +
> +struct tc358764 {
> +	struct device *dev;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct regulator_bulk_data supplies[ARRAY_SIZE(tc358764_supplies)];
> +	struct gpio_desc *gpio_reset;
> +
> +	struct drm_panel *panel;
> +};
> +
> +static int tc358764_read(struct tc358764 *ctx, u16 addr, u32 *val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM,
> +		.tx_buf = &addr,
> +		.tx_len = 2,
> +		.rx_buf = val,
> +		.rx_len = 4
> +	};
> +	ssize_t ret;
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	cpu_to_le16s(&addr);
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +	if (ret >= 0)
> +		le32_to_cpus(val); > +
> +	dev_dbg(ctx->dev, "read: %d, addr: %d\n", addr, *val);
> +
> +	return ret;
> +}
> +
> +static int tc358764_write(struct tc358764 *ctx, u16 addr, u32 val)
> +{
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	const struct mipi_dsi_host_ops *ops = dsi->host->ops;
> +	u8 data[6];
> +	int ret;
> +	struct mipi_dsi_msg msg = {
> +		.type = MIPI_DSI_GENERIC_LONG_WRITE,
> +		.channel = dsi->channel,
> +		.flags = MIPI_DSI_MSG_USE_LPM | MIPI_DSI_MSG_REQ_ACK,
> +		.tx_buf = data,
> +		.tx_len = 6
> +	};
> +
> +	if (!ops || !ops->transfer)
> +		return -EINVAL;
> +
> +	data[0] = addr;
> +	data[1] = addr >> 8;
> +	data[2] = val;
> +	data[3] = val >> 8;
> +	data[4] = val >> 16;
> +	data[5] = val >> 24;
> +
> +	ret = ops->transfer(dsi->host, &msg);
> +
> +	return ret;
> +}
> +
> +static inline struct tc358764 *bridge_to_tc358764(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tc358764, bridge);
> +}
> +
> +static inline
> +struct tc358764 *connector_to_tc358764(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct tc358764, connector);
> +}
> +
> +static int tc358764_init(struct tc358764 *ctx)
> +{
> +	u32 v = 0;
> +
> +	tc358764_read(ctx, SYS_ID, &v);
> +	dev_info(ctx->dev, "ID: %#x\n", v);
> +
> +	/* configure PPI counters */
> +	tc358764_write(ctx, PPI_TX_RX_TA, TTA_GET | TTA_SURE);
> +	tc358764_write(ctx, PPI_LPTXTIMECNT, LPX_PERIOD);
> +	tc358764_write(ctx, PPI_D0S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D1S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D2S_CLRSIPOCOUNT, 5);
> +	tc358764_write(ctx, PPI_D3S_CLRSIPOCOUNT, 5);
> +
> +	/* enable four data lanes and clock lane */
> +	tc358764_write(ctx, PPI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +	tc358764_write(ctx, DSI_LANEENABLE, LANEENABLE_L3EN | LANEENABLE_L2EN |
> +		       LANEENABLE_L1EN | LANEENABLE_L0EN | LANEENABLE_CLEN);
> +
> +	/* start */
> +	tc358764_write(ctx, PPI_STARTPPI, PPI_START_FUNCTION);
> +	tc358764_write(ctx, DSI_STARTDSI, DSI_RX_START);
> +
> +	/* configure video path */
> +	tc358764_write(ctx, VP_CTRL, VP_CTRL_VSDELAY(15) | VP_CTRL_RGB888(1) |
> +		       VP_CTRL_EVTMODE(1) | VP_CTRL_HSPOL | VP_CTRL_VSPOL);

Could we specify somewhere that
> +
> +	/* reset PHY */
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_RST(1) |
> +		       LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) | LV_PHY0_ND(6));
> +	tc358764_write(ctx, LV_PHY0, LV_PHY0_PRBS_ON(4) | LV_PHY0_IS(2) |
> +		       LV_PHY0_ND(6));
> +
> +	/* reset bridge */
> +	tc358764_write(ctx, SYS_RST, SYS_RST_LCD);
> +
> +	/* set bit order */
> +	tc358764_write(ctx, LV_MX0003, LV_MX(LVI_R0, LVI_R1, LVI_R2, LVI_R3));
> +	tc358764_write(ctx, LV_MX0407, LV_MX(LVI_R4, LVI_R7, LVI_R5, LVI_G0));
> +	tc358764_write(ctx, LV_MX0811, LV_MX(LVI_G1, LVI_G2, LVI_G6, LVI_G7));
> +	tc358764_write(ctx, LV_MX1215, LV_MX(LVI_G3, LVI_G4, LVI_G5, LVI_B0));
> +	tc358764_write(ctx, LV_MX1619, LV_MX(LVI_B6, LVI_B7, LVI_B1, LVI_B2));
> +	tc358764_write(ctx, LV_MX2023, LV_MX(LVI_B3, LVI_B4, LVI_B5, LVI_L0));
> +	tc358764_write(ctx, LV_MX2427, LV_MX(LVI_HS, LVI_VS, LVI_DE, LVI_R6));
> +	tc358764_write(ctx, LV_CFG, LV_CFG_CLKPOL2 | LV_CFG_CLKPOL1 |
> +		       LV_CFG_LVEN);
> +
> +	return 0;
> +}
> +
> +static void tc358764_reset(struct tc358764 *ctx)
> +{
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 0);
> +	msleep(20);
> +	gpiod_set_value(ctx->gpio_reset, 1);
> +	msleep(40);
> +}
> +
> +static void tc358764_poweroff(struct tc358764 *ctx)
> +{
> +	int ret;
> +
> +	tc358764_reset(ctx);
> +
> +	drm_panel_disable(ctx->panel);
> +	msleep(40);
> +
> +	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error disabling regulators (%d)\n", ret);
> +}
> +
> +static int tc358764_get_modes(struct drm_connector *connector)
> +{
> +	struct tc358764 *ctx = connector_to_tc358764(connector);
> +
> +	if (ctx->panel && ctx->panel->funcs && ctx->panel->funcs->get_modes)
> +		return ctx->panel->funcs->get_modes(ctx->panel);
> +
> +	return 0;
> +}
> +
> +static const
> +struct drm_connector_helper_funcs tc358764_connector_helper_funcs = {
> +	.get_modes = tc358764_get_modes,
> +};
> +
> +static const struct drm_connector_funcs tc358764_connector_funcs = {
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.reset = drm_atomic_helper_connector_reset,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> +};
> +
> +static void tc358764_disable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +
> +	tc358764_poweroff(ctx);
> +}
> +
> +static void tc358764_pre_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies),
> +					ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "error enabling regulators (%d)\n", ret);
> +
> +	tc358764_reset(ctx);
> +	tc358764_init(ctx);
> +}
> +
> +static void tc358764_enable(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	int ret;
> +
> +	drm_panel_prepare(ctx->panel);
> +
> +	ret = drm_panel_enable(ctx->panel);
> +	if (ret < 0)
> +		pr_err("panel enable failed\n");
> +
> +	msleep(40);
> +}
> +
> +static int tc358764_attach(struct drm_bridge *bridge)
> +{
> +	struct tc358764 *ctx = bridge_to_tc358764(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	if (!bridge->encoder) {
> +		DRM_ERROR("Encoder not found\n");
> +		return -ENODEV;
> +	}
> +
> +	ctx->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	ret = drm_connector_init(drm, &ctx->connector,
> +				 &tc358764_connector_funcs,
> +				 DRM_MODE_CONNECTOR_LVDS);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(&ctx->connector,
> +				 &tc358764_connector_helper_funcs);
> +
> +	drm_mode_connector_attach_encoder(&ctx->connector, bridge->encoder);
> +
> +	if (ctx->panel)
> +		drm_panel_attach(ctx->panel, &ctx->connector);
> +
> +	drm_atomic_helper_connector_reset(&ctx->connector);
> +	drm_connector_register(&ctx->connector);
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs tc358764_bridge_funcs = {
> +	.disable = tc358764_disable,
> +	.enable = tc358764_enable,
> +	.pre_enable = tc358764_pre_enable,
> +	.attach = tc358764_attach,
> +};
> +
> +static struct device_node *tc358764_of_find_panel_node(struct device *dev)
> +{
> +	struct device_node *np, *ep;
> +
> +	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
> +	if (!ep) {
> +		pr_err("faile to get endpoint\n");
> +		return NULL;
> +	}
> +
> +	np = of_graph_get_remote_port_parent(ep);
> +
> +	return np;
> +}
> +
> +static int tc358764_parse_dt(struct tc358764 *ctx)
> +{
> +	struct device *dev = ctx->dev;
> +	struct device_node *np = dev->of_node;
> +	struct device_node *lvds;
> +
> +	ctx->gpio_reset = devm_gpiod_get_from_of_node(dev, np, "reset", 0,
> +						      GPIOD_OUT_LOW,
> +						      "tc358764-reset");
> +	if (IS_ERR(ctx->gpio_reset)) {
> +		dev_err(dev, "no reset GPIO pin provided\n");
> +		return PTR_ERR(ctx->gpio_reset);
> +	}
> +
> +	lvds = tc358764_of_find_panel_node(ctx->dev);
> +	if (!lvds) {
> +		dev_err(dev, "cannot find panel node\n");
> +		return -EINVAL;
> +	}
> +
> +	ctx->panel = of_drm_find_panel(lvds);
> +	if (!ctx->panel) {
> +		dev_err(dev, "panel not registered\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tc358764_configure_regulators(struct tc358764 *ctx)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx->supplies); ++i)
> +		ctx->supplies[i].supply = tc358764_supplies[i];
> +
> +	ret = devm_regulator_bulk_get(ctx->dev, ARRAY_SIZE(ctx->supplies),
> +				      ctx->supplies);
> +	if (ret < 0)
> +		dev_err(ctx->dev, "failed to get regulators: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int tc358764_probe(struct mipi_dsi_device *dsi)
> +{
> +	struct device *dev = &dsi->dev;
> +	struct tc358764 *ctx;
> +	int ret;
> +
> +	ctx = devm_kzalloc(dev, sizeof(struct tc358764), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	mipi_dsi_set_drvdata(dsi, ctx);
> +
> +	ctx->dev = dev;
> +
> +	dsi->lanes = 4;
> +	dsi->format = MIPI_DSI_FMT_RGB888;
> +	dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_VIDEO_BURST
> +		| MIPI_DSI_MODE_VIDEO_AUTO_VERT;

if you use the mipi_dsi_device_transfer() helper, I guess you'd need to
add the LPM flag here.

Looks good to me otherwise. Once the DT port issue is concluded:

Reviewed-by: Archit Taneja <architt@codeaurora.org>

> +
> +	ret = tc358764_parse_dt(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = tc358764_configure_regulators(ctx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ctx->bridge.funcs = &tc358764_bridge_funcs;
> +	ctx->bridge.of_node = dev->of_node;
> +
> +	drm_bridge_add(&ctx->bridge);
> +
> +	ret = mipi_dsi_attach(dsi);
> +	if (ret < 0) {
> +		drm_bridge_remove(&ctx->bridge);
> +		dev_err(dev, "failed to attach dsi\n");
> +	}
> +
> +	return ret;
> +}
> +
> +static int tc358764_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct tc358764 *ctx = mipi_dsi_get_drvdata(dsi);
> +
> +	tc358764_poweroff(ctx);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_bridge_remove(&ctx->bridge);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id tc358764_of_match[] = {
> +	{ .compatible = "toshiba,tc358764" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, tc358764_of_match);
> +
> +static struct mipi_dsi_driver tc358764_driver = {
> +	.probe = tc358764_probe,
> +	.remove = tc358764_remove,
> +	.driver = {
> +		.name = "tc358764",
> +		.owner = THIS_MODULE,
> +		.of_match_table = tc358764_of_match,
> +	},
> +};
> +module_mipi_dsi_driver(tc358764_driver);
> +
> +MODULE_AUTHOR("Andrzej Hajda <a.hajda@samsung.com>");
> +MODULE_AUTHOR("Maciej Purski <m.purski@samsung.com>");
> +MODULE_DESCRIPTION("MIPI-DSI based Driver for TC358764 DSI/LVDS Bridge");
> +MODULE_LICENSE("GPL v2");
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v8 5/7] i2c: fsi: Add transfer implementation
From: Andy Shevchenko @ 2018-05-31  6:50 UTC (permalink / raw)
  To: Eddie James
  Cc: linux-i2c, Linux Kernel Mailing List, devicetree, Wolfram Sang,
	Rob Herring, Benjamin Herrenschmidt, Joel Stanley, Mark Rutland,
	Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <1527714464-8642-6-git-send-email-eajames@linux.vnet.ibm.com>

On Thu, May 31, 2018 at 12:07 AM, Eddie James
<eajames@linux.vnet.ibm.com> wrote:
> Execute I2C transfers from the FSI-attached I2C master. Use polling
> instead of interrupts as we have no hardware IRQ over FSI.
>

Few nitpicks, after addressing them (field definition is up to you though)

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Signed-off-by: Eddie James <eajames@linux.vnet.ibm.com>
> ---
>  drivers/i2c/busses/i2c-fsi.c | 196 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 194 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index e5dd0f7..f24e4b9 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -149,6 +149,7 @@ struct fsi_i2c_port {
>         struct i2c_adapter      adapter;
>         struct fsi_i2c_master   *master;
>         u16                     port;
> +       u16                     xfrd;
>  };
>
>  static int fsi_i2c_read_reg(struct fsi_device *fsi, unsigned int reg,
> @@ -224,6 +225,100 @@ static int fsi_i2c_set_port(struct fsi_i2c_port *port)
>         return fsi_i2c_write_reg(fsi, I2C_FSI_RESET_ERR, &dummy);
>  }
>
> +static int fsi_i2c_start(struct fsi_i2c_port *port, struct i2c_msg *msg,
> +                        bool stop)
> +{
> +       struct fsi_i2c_master *i2c = port->master;
> +       u32 cmd = I2C_CMD_WITH_START | I2C_CMD_WITH_ADDR;
> +
> +       port->xfrd = 0;
> +

> +       if (msg->flags & I2C_M_RD)
> +               cmd |= I2C_CMD_READ;

(1)

> +
> +       if (stop || msg->flags & I2C_M_STOP)
> +               cmd |= I2C_CMD_WITH_STOP;
> +

> +       cmd |= FIELD_PREP(I2C_CMD_ADDR, msg->addr >> 1);

(1) + this looks like:

 // #define I2C_CMD_ADDR           GENMASK(23, 17)
 // #define I2C_CMD_READ           BIT(16)

#define I2C_CMD_8BIT_ADDR           GENMASK(23, 16)
// or even drop I2C_CMD_READ completely

   cmd |= FIELD_PREP(I2C_CMD_8BIT_ADDR, i2c_8bit_addr_from_msg(msg));

> +       cmd |= FIELD_PREP(I2C_CMD_LEN, msg->len);
> +
> +       return fsi_i2c_write_reg(i2c->fsi, I2C_FSI_CMD, &cmd);
> +}
> +
> +static int fsi_i2c_get_op_bytes(int op_bytes)
> +{
> +       /* fsi is limited to max 4 byte aligned ops */
> +       if (op_bytes > 4)
> +               return 4;
> +       else if (op_bytes == 3)
> +               return 2;
> +       else
> +               return op_bytes;

If you have pattern like

if {
 ...
 return;
} else {
 ...
}

'else' is redundant.

> +}

Overall, I think rounddown_pow_of_two() makes sense here.
Though it's your choice.

> +
> +static int fsi_i2c_write_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
> +                             u8 fifo_count)
> +{
> +       int write;
> +       int rc;
> +       struct fsi_i2c_master *i2c = port->master;
> +       int bytes_to_write = i2c->fifo_size - fifo_count;
> +       int bytes_remaining = msg->len - port->xfrd;
> +
> +       bytes_to_write = min(bytes_to_write, bytes_remaining);
> +
> +       while (bytes_to_write) {
> +               write = fsi_i2c_get_op_bytes(bytes_to_write);
> +
> +               rc = fsi_device_write(i2c->fsi, I2C_FSI_FIFO,
> +                                     &msg->buf[port->xfrd], write);
> +               if (rc)
> +                       return rc;
> +
> +               port->xfrd += write;
> +               bytes_to_write -= write;
> +       }
> +
> +       return 0;
> +}
> +
> +static int fsi_i2c_read_fifo(struct fsi_i2c_port *port, struct i2c_msg *msg,
> +                            u8 fifo_count)
> +{
> +       int read;
> +       int rc;
> +       struct fsi_i2c_master *i2c = port->master;
> +       int bytes_to_read;
> +       int xfr_remaining = msg->len - port->xfrd;
> +       u32 dummy;
> +
> +       bytes_to_read = min_t(int, fifo_count, xfr_remaining);
> +
> +       while (bytes_to_read) {
> +               read = fsi_i2c_get_op_bytes(bytes_to_read);
> +
> +               if (xfr_remaining) {
> +                       rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO,
> +                                            &msg->buf[port->xfrd], read);
> +                       if (rc)
> +                               return rc;
> +
> +                       port->xfrd += read;
> +                       xfr_remaining -= read;
> +               } else {
> +                       /* no more buffer but data in fifo, need to clear it */
> +                       rc = fsi_device_read(i2c->fsi, I2C_FSI_FIFO, &dummy,
> +                                            read);
> +                       if (rc)
> +                               return rc;
> +               }
> +
> +               bytes_to_read -= read;
> +       }
> +
> +       return 0;
> +}
> +
>  static int fsi_i2c_reset_bus(struct fsi_i2c_master *i2c)
>  {
>         int i, rc;
> @@ -387,17 +482,114 @@ static int fsi_i2c_abort(struct fsi_i2c_port *port, u32 status)
>         return -ETIMEDOUT;
>  }
>
> +static int fsi_i2c_handle_status(struct fsi_i2c_port *port,
> +                                struct i2c_msg *msg, u32 status)
> +{
> +       int rc;
> +       u8 fifo_count;
> +
> +       if (status & I2C_STAT_ERR) {
> +               rc = fsi_i2c_abort(port, status);
> +               if (rc)
> +                       return rc;
> +
> +               if (status & I2C_STAT_INV_CMD)
> +                       return -EINVAL;
> +
> +               if (status & (I2C_STAT_PARITY | I2C_STAT_BE_OVERRUN |
> +                   I2C_STAT_BE_ACCESS))
> +                       return -EPROTO;
> +
> +               if (status & I2C_STAT_NACK)
> +                       return -ENXIO;
> +
> +               if (status & I2C_STAT_LOST_ARB)
> +                       return -EAGAIN;
> +
> +               if (status & I2C_STAT_STOP_ERR)
> +                       return -EBADMSG;
> +
> +               return -EIO;
> +       }
> +
> +       if (status & I2C_STAT_DAT_REQ) {
> +               fifo_count = FIELD_GET(I2C_STAT_FIFO_COUNT, status);
> +
> +               if (msg->flags & I2C_M_RD)
> +                       return fsi_i2c_read_fifo(port, msg, fifo_count);

> +               else
> +                       return fsi_i2c_write_fifo(port, msg, fifo_count);

Redundant 'else'

> +       }
> +
> +       if (status & I2C_STAT_CMD_COMP) {
> +               if (port->xfrd < msg->len)
> +                       return -ENODATA;

> +               else
> +                       return msg->len;

Ditto.

> +       }
> +
> +       return 0;
> +}
> +
> +static int fsi_i2c_wait(struct fsi_i2c_port *port, struct i2c_msg *msg,
> +                       unsigned long timeout)
> +{
> +       u32 status = 0;
> +       int rc;
> +       unsigned long start = jiffies;
> +
> +       do {
> +               rc = fsi_i2c_read_reg(port->master->fsi, I2C_FSI_STAT,
> +                                     &status);
> +               if (rc)
> +                       return rc;
> +
> +               if (status & I2C_STAT_ANY_RESP) {
> +                       rc = fsi_i2c_handle_status(port, msg, status);
> +                       if (rc < 0)
> +                               return rc;
> +
> +                       /* cmd complete and all data xfrd */
> +                       if (rc == msg->len)
> +                               return 0;
> +
> +                       /* need to xfr more data, but maybe don't need wait */
> +                       continue;
> +               }
> +
> +               usleep_range(I2C_CMD_SLEEP_MIN_US, I2C_CMD_SLEEP_MAX_US);
> +       } while (time_after(start + timeout, jiffies));
> +
> +       return -ETIMEDOUT;
> +}
> +
>  static int fsi_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
>                         int num)
>  {
> -       int rc;
> +       int i, rc;
> +       unsigned long start_time;
>         struct fsi_i2c_port *port = adap->algo_data;
> +       struct i2c_msg *msg;
>
>         rc = fsi_i2c_set_port(port);
>         if (rc)
>                 return rc;
>
> -       return -EOPNOTSUPP;
> +       for (i = 0; i < num; ++i) {

i++

> +               msg = msgs + i;
> +               start_time = jiffies;
> +
> +               rc = fsi_i2c_start(port, msg, i == num - 1);
> +               if (rc)
> +                       return rc;
> +
> +               rc = fsi_i2c_wait(port, msg,
> +                                 adap->timeout - (jiffies - start_time));
> +               if (rc)
> +                       return rc;
> +       }
> +
> +       return 0;
>  }
>
>  static u32 fsi_i2c_functionality(struct i2c_adapter *adap)
> --
> 1.8.3.1
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH 00/12] introduce support for early platform drivers
From: Geert Uytterhoeven @ 2018-05-31  6:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Michael Turquette, Bartosz Golaszewski, Sekhar Nori, Kevin Hilman,
	David Lechner, Stephen Boyd, Arnd Bergmann, Greg Kroah-Hartman,
	Mark Rutland, Yoshinori Sato, Rich Felker, Andy Shevchenko,
	Marc Zyngier, Rafael J . Wysocki, Peter Rosin, Jiri Slaby,
	Thomas Gleixner, Daniel Lezcano, Magnus Damm, Johan Hovold
In-Reply-To: <CAL_Jsq+7=NGj57k=E5GAuDmW7Uu_YkHJXf8jSyc_ad3r1op4yA@mail.gmail.com>

Hi Rob,

On Thu, May 31, 2018 at 12:36 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, May 30, 2018 at 2:40 PM, Michael Turquette
> <mturquette@baylibre.com> wrote:
>> Quoting Rob Herring (2018-05-14 06:20:57)
>>> On Mon, May 14, 2018 at 6:38 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> > 2018-05-11 22:13 GMT+02:00 Rob Herring <robh+dt@kernel.org>:
>>> >> On Fri, May 11, 2018 at 11:20 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> >>> This series is a follow-up to the RFC[1] posted a couple days ago.
>>> >>>
>>> >>> NOTE: this series applies on top of my recent patches[2] that move the previous
>>> >>> implementation of early platform devices to arch/sh.
>>> >>>
>>> >>> Problem:
>>> >>>
>>> >>> Certain class of devices, such as timers, certain clock drivers and irq chip
>>> >>> drivers need to be probed early in the boot sequence. The currently preferred
>>> >>> approach is using one of the OF_DECLARE() macros. This however does not create
>>> >>> a platform device which has many drawbacks - such as not being able to use
>>> >>> devres routines, dev_ log functions or no way of deferring the init OF function
>>> >>> if some other resources are missing.
>>> >>
>>> >> I skimmed though this and it doesn't look horrible (how's that for
>>> >> positive feedback? ;) ). But before going into the details, I think
>>> >> first there needs to be agreement this is the right direction.
>>> >>
>>> >> The question does remain though as to whether this class of devices
>>> >> should be platform drivers. They can't be modules. They can't be
>>> >> hotplugged. Can they be runtime-pm enabled? So the advantage is ...
>>> >>
>>> >
>>> > The main (but not the only) advantage for drivers that can both be
>>> > platform drivers and OF_DECLARE drivers is that we get a single entry
>>> > point and can reuse code without resorting to checking if (!dev). It
>>> > results in more consistent code base. Another big advantage is
>>> > consolidation of device tree and machine code for SoC drivers used in
>>> > different boards of which some are still using board files and others
>>> > are defined in DT (see: DaVinci).
>>> >
>>> >> I assume that the clock maintainers had some reason to move clocks to
>>> >> be platform drivers. It's just not clear to me what that was.
>>> >>
>>> >>> For drivers that use both platform drivers and OF_DECLARE the situation is even
>>> >>> more complicated as the code needs to take into account that there can possibly
>>> >>> be no struct device present. For a specific use case that we're having problems
>>> >>> with, please refer to the recent DaVinci common-clock conversion patches and
>>> >>> the nasty workaround that this problem implies[3].
>>> >>
>>> >> So devm_kzalloc will work with this solution? Why did we need
>>> >> devm_kzalloc in the first place? The clocks can never be removed and
>>> >> cleaning up on error paths is kind of pointless. The system would be
>>> >> hosed, right?
>>> >>
>>> >
>>> > It depends - not all clocks are necessary for system to boot.
>>>
>>> That doesn't matter. You have a single driver for all/most of the
>>> clocks, so the driver can't be removed.
>>
>> -ECANOFWORMS
>>
>> A couple of quick rebuttals, but I imagine we're going to disagree on
>> the platform_driver thing as a matter of taste no matter what...
>
> It's really more should the clocksource, clockevents and primary
> interrupt controller be drivers. Let's get agreement on that first. If
> yes, then it probably does make sense that their dependencies are
> drivers too. If not, then making only the dependencies drivers doesn't
> seem right to me.

Yes, they should all be drivers.

Assuming clocksources, clockevents, or primary interrupt controllers are
special, and thus forcing everyone to use OF_DECLARE for the whole
subsystem, doesn't fly everywhere.

Clockevents and interrupt controllers can have a module clock.
All three can be part of a PM Domain, which requires a struct device to
be handled properly.

>> 1) There should be multiple clk drivers in a properly modeled system.
>> Some folks still incorrectly put all clocks in a system into a single
>> driver because That's How We Used To Do It, and some systems (simpler
>> ones) really only have a single clock generator IP block.
>>
>> Excepting those two reasons above, we really should have separate
>> drivers for clocks controlled by the PMIC, for the (one or more) clock
>> generator blocks inside of the AP/SoC, and then even more for the
>> drivers that map to IP blocks that have their own clock gens.
>
> I agree those should be separate entities at least. But for a given
> h/w block, if you already have to use OF_DECLARE, why would you try to
> split that into OF_DECLARE and a driver? what advantage does putting
> non-boot essential clocks in a driver or transitioning to a driver get
> you?

You may want to split it because of dependencies. OF_DECLARE
doesn't handle EPROBE_DEFER, while some critical parts may be
needed early.

> And I've seen PMIC clocks could be inputs to the SoC's clock
> controller(s), so the dependencies get more complicated. Then does the
> PMIC driver and its dependencies need to be early drivers too?

Especially if there are clock loops, but that's something different.
Cfr. cs2000 in arch/arm64/boot/dts/renesas/salvator-common.dtsi, which
provides a clock from the main SoC, but also consumes a clock from the main
SoC. The latter is modeled in DT as a fixed-clock as a workaround.

>> Good examples of the latter are display controllers that generate their
>> own PLL and pixel clock. Or MMC controllers that have a
>> runtime-programmable clock divider. Examples of these are merged into
>> mainline.
>
> But those are drivers of types other than a clock controller that
> happen to register some clocks as well. I wasn't saying these cases
> can't or shouldn't be part of the driver model. Look at irqchips. We
> have some that use the driver model (e.g. every GPIO driver) and some
> that don't because there's no need (e.g. GIC).

There is a need for using the driver model for the GIC. On some platforms,
the GIC can be part of a clock and/or power domain.

For secondary GICs, that got fixed in commit 9c8edddfc9924cb4
("irqchip/gic: Add platform driver for non-root GICs that require RPM").
For primary GICs, it's still not fixed, so we have to live with
CLK_IS_CRITICAL, and rely on other critical devices in the same power
domain to avoid the power domain being powered off.

>> 2) Stephen and I wanted clock drivers to actually be represented in the
>> driver model. There were these gigantic clock drivers that exclusively
>> used CLK_OF_DECLARE and they just sort of floated out there in the
>> ether... no representation in sysfs, no struct device to map onto a
>> clock controller struct, etc.
>>
>> I'd be happy to hear why you think that platform_driver is a bad fit for
>> a device driver that generally manages memory-mapped system resources
>> that are part of the system glue and not really tied to a specific bus.
>> Sounds like a good fit to me.
>>
>> If platform_driver doesn't handle the early boot thing well, that tells
>> me that we have a problem to solve in platform_driver, not in the clk
>> subsystem or drivers.
>
> Doing things earlier is not the only way to solve the problems.
> Perhaps we need to figure out how to start things later. Maybe it's
> not feasible here, I don't know.

The fixed probe order imposed by OF_DECLARE() limits this: if your
OF_DECLARE() driver depends on something else, the latter must become an
early device.
If all subsystems would use real devices, EPROBE_DEFER would handle most of
it automatically.

Then, next step would be to avoid triggering EPROBE_DEFER, e.g. by
analyzing dependencies in DT...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 2/6] mtd: rawnand: tegra: add devicetree binding
From: Miquel Raynal @ 2018-05-31  6:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Stefan Agner, boris.brezillon, dwmw2, computersforpeace,
	marek.vasut, mark.rutland, thierry.reding, mturquette, sboyd, dev,
	richard, marcel, krzk, digetx, benjamin.lindqvist, jonathanh,
	pdeschrijver, pgaikwad, mirza.krak, linux-mtd, linux-tegra,
	devicetree, linux-kernel, linux-clk
In-Reply-To: <20180531034523.GA5417@rob-hp-laptop>

Hi Rob,

On Wed, 30 May 2018 22:45:23 -0500, Rob Herring <robh@kernel.org> wrote:

> On Mon, May 28, 2018 at 12:23:54AM +0200, Miquel Raynal wrote:
> > Hi Stefan,
> > 
> > On Sun, 27 May 2018 23:54:38 +0200, Stefan Agner <stefan@agner.ch>
> > wrote:
> >   
> > > From: Lucas Stach <dev@lynxeye.de>
> > > 
> > > This adds the devicetree binding for the Tegra 2 NAND flash
> > > controller.
> > > 
> > > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > > ---
> > >  .../bindings/mtd/nvidia,tegra20-nand.txt      | 62 +++++++++++++++++++
> > >  1 file changed, 62 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
> > > new file mode 100644
> > > index 000000000000..49e472af1b39
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/nvidia,tegra20-nand.txt
> > > @@ -0,0 +1,62 @@
> > > +NVIDIA Tegra NAND Flash controller
> > > +
> > > +Required properties:
> > > +- compatible: Must be one of:  
> > 
> > Nitpick: just put the compatible here as there is only one?  
> 
> No, this is how I ask people to format it.

Oh, ok, I'll remember it for my own DT patches then :)

Thanks,
Miquèl

^ permalink raw reply

* Re: [PATCH v8 0/7] i2c: Add FSI-attached I2C master algorithm
From: Andy Shevchenko @ 2018-05-31  6:29 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Eddie James, linux-i2c, Linux Kernel Mailing List, devicetree,
	Wolfram Sang, Rob Herring, Joel Stanley, Mark Rutland,
	Greg Kroah-Hartman, Randy Dunlap
In-Reply-To: <58c1059a91b93a490a7fc8bda2112e67e6513840.camel@kernel.crashing.org>

On Thu, May 31, 2018 at 1:42 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2018-05-31 at 00:31 +0300, Andy Shevchenko wrote:
>> On Thu, May 31, 2018 at 12:07 AM, Eddie James
>> <eajames@linux.vnet.ibm.com> wrote:

>> I'll comment the series later, though you have to address previous
>> comments first:
>> - understand devm_ purpose and how it works
>
> I think it is perfectly understood and I don't see what your problem
> here is. So please be a proper civil human being an express your
> concern precisely rather than with aggressive comments.

I apologize for this kind of tone, let's assume it was a bad day.

> Now to clarify that specific point, devm purpose is to automatically
> clean up the resources used by the device when it is torn down.
>
> However, in this specific case, it makes sense to dispose of the port
> structure explicitly because this is a failure in registering an
> individual port which doesn't lead to a failure of the entire driver.
>
> Thus not freeing it means the structure would remain allocated
> uselessly until the whole driver is torn down.

Yep, so, why do we care? If it holds few hundreds of bytes, can't we
live with it?
If no, the devm_k*alloc() is a wrong choice in the first place.

>> - discuss with maintainer a design of enumerating ports
>
> I've been at that game for at least a good 2 decades. Maintainers
> generally do *not* discuss design until a patch is proposed. I even
> still try every now and then, maintainers are like lawyers, they don't
> want to tell you what to do in case they still want to reject it after
> seeing it later :-) I know I've been one of them for long enough.
>
> If you have specific issues with how this is done, please express them
> clearly. It's quite possible that there's some better way to do what
> Eddie is doing here, but without *construtive* feedback this is
> pointless.

It feels like you duplicate approach which is done in OF generic case.
That is my concern. Though, if Wolfram is telling that is OK, I have
no objections.

> I'm disappointed here because we have an example of somebody rather new
> producing what is overall pretty damn good code,

That is true. His code much better than many I have seen before.

> despite a few corner
> issues, and being (again) treated like crap.

Sorry for that, life is harsh.

> This isn't the right way to operate, and I believe this has been made
> clear many times before.

Yes.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v7 3/7] drivers/i2c: Add port structure to FSI algorithm
From: Andy Shevchenko @ 2018-05-31  6:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Eddie James, linux-i2c, Linux Kernel Mailing List, devicetree,
	Wolfram Sang, Rob Herring, Joel Stanley, Mark Rutland,
	Greg Kroah-Hartman, Edward A. James
In-Reply-To: <48444d16bd410188bd43ca5eea97f03f70c80f42.camel@kernel.crashing.org>

On Thu, May 31, 2018 at 1:34 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2018-05-31 at 00:27 +0300, Andy Shevchenko wrote:
>> On Wed, May 30, 2018 at 6:47 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> > On 05/29/2018 06:19 PM, Andy Shevchenko wrote:
>> > > On Wed, May 30, 2018 at 1:24 AM, Eddie James <eajames@linux.vnet.ibm.com>
>> > > wrote:

>> > > Isn't below somehow repeats of_i2c_register_devices() ?
>> > > Why not to use it?

>> > Because I need to assign all these port structure fields. Also looks like
>> > of_i2c_register_devices creates new devices; I just want an adapter for each
>> > port.
>>
>> Hmm... Wolfram, what is your opinion on this design?
>
> Andy, I don't understand your issue.
>
> of_i2c_register_devices() is about discovering the i2c devices below a
> given bus. This is not what is happening here.
>
> This is a driver for a master that supports multiple busses, so it the
> above loop creates all the busses.

My issue here, that it feels like a lot of duplication with existing approaches.
Though, it might be a right thing to do at the end. So, let's just
assume maintainer will give their point of view.

>> > > > +                       devm_kfree(dev, port);
>> > >
>> > > This hurts my eyes. Why?!
>> > What would you suggest instead?
>>
>> You even didn't wait for answer, why to ask then?
>
> Please stop being so rude.

OK.

>> Moreover, you didn't answer to my question. Why are you doing that
>> call implicitly?
>
> "implicitly" ? What's implicit here ? This is just pretty standard
> cleanup after failure, you are being very cryptic here.
>
> Please state precisely what it is you dislike with that code instead of
> expecting us to guess and being nasty about it. Eddie was a genuine
> question, he doesn't see what you think is "hurtful to the eyes" in the
> code you quoted.

In 99% cases when someone calls devm_kfree() it means wrong choice of
devm_k*alloc() in the first place.
So, with explanation given why it's done in this way I would rather
suggest to switch to plain k*alloc() / kfree().

Or do we really care about few hundreds of bytes wasted?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 5/9] PM / Domains: dt: Allow power-domain property to be a list of specifiers
From: Viresh Kumar @ 2018-05-31  6:18 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J . Wysocki, linux-pm, Greg Kroah-Hartman, Jon Hunter,
	Geert Uytterhoeven, Todor Tomov, Rajendra Nayak, Vincent Guittot,
	Kevin Hilman, linux-kernel, linux-arm-kernel, linux-tegra,
	Rob Herring, devicetree
In-Reply-To: <20180529100421.31022-6-ulf.hansson@linaro.org>

On 29-05-18, 12:04, Ulf Hansson wrote:
> To be able to describe topologies where devices are partitioned across
> multiple power domains, let's extend the power-domain property to allow
> being a list of PM domain specifiers.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: devicetree@vger.kernel.org
> Suggested-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

^ permalink raw reply

* Re: [PATCH v2 1/6] ARM: dra762: hwmod: Add MCAN support
From: Tero Kristo @ 2018-05-31  6:15 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Faiz Abbas, linux-kernel, devicetree, linux-omap,
	linux-arm-kernel, linux-clk, robh+dt, bcousson, paul
In-Reply-To: <20180530155448.GH5705@atomide.com>

On 30/05/18 18:54, Tony Lindgren wrote:
> * Tero Kristo <t-kristo@ti.com> [180530 15:44]:
>> On 30/05/18 18:28, Tony Lindgren wrote:
>>> * Tero Kristo <t-kristo@ti.com> [180530 15:18]:
>>>> For the OCP if part, I think that is still needed until we switch over to
>>>> full sysc driver. clkctrl_offs you probably also need because that is used
>>>> for mapping the omap_hwmod against a specific clkctrl clock. Those can be
>>>> only removed once we are done with hwmod (or figure out some other way to
>>>> assign the clkctrl clock to a hwmod.)
>>>
>>> Hmm might be worth testing. I thought your commit 70f05be32133
>>> ("ARM: OMAP2+: hwmod: populate clkctrl clocks for hwmods if available")
>>> already parses the clkctrl from dts?
>>
>> It maps the clkctrl clock to be used by hwmod, if those are available. We
>> didn't add any specific clock entries to DT for mapping the actual clkctrl
>> clock without the hwmod_data hints yet though, as that was deemed temporary
>> solution only due to transition to interconnect driver. I.e., you would need
>> something like this in DT for every device node:
>>
>> &uart3 {
>>    clocks = <l4per_clkctrl UART3_CLK 0>;
>>    clock-names = "clkctrl";
>> };
>>
>> ... which is currently not present.
> 
> Hmm is that not the "fck" clkctrl clock we have already in
> the dts files for the interconnect target modules?

Oh okay, yeah, we could parse that one, but currently it is not done, 
and is not present for everything either I believe.

> We can also use pdata callbacks to pass the clock node if
> needed. But I guess I don't quite still understand what we
> are missing :)

So, what is missing is the glue logic only from the hwmod codebase. 
Right now this is not supported but should be relatively trivial thing 
to add if we really want to do this.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH v6] usb: gadget: udc: renesas_usb3: Add register of usb role switch
From: Yoshihiro Shimoda @ 2018-05-31  6:11 UTC (permalink / raw)
  To: balbi, robh+dt, mark.rutland
  Cc: gregkh, heikki.krogerus, hdegoede, andy.shevchenko, linux-usb,
	linux-renesas-soc, devicetree, Yoshihiro Shimoda

This patch adds role switch support for R-Car SoCs into the USB 3.0
peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0
dual-role device controller which has the USB 3.0 xHCI host and
Renesas USB 3.0 peripheral.

Unfortunately, the mode change register (DRD_CON) contains
the USB 3.0 peripheral controller side only. So, this renesas_usb3
driver manages the DRD_CON now. However, in peripheral mode, the host
should stop. Also the host hardware needs to reinitialize its own
registers when the mode changes from peripheral to host mode.
Otherwise, the host cannot work correctly (e.g. detect a device
as high-speed).

To achieve this reinitialization by a driver, this driver also
registers a role switch driver to manage the DRD_CON and get
a device pointer of usb 3.0 host from "companion" property of OF.
Then, when the usb role is changed, renesas_usb3_role_switch_set()
will attach/release the xhci-plat driver to reinitialize the host
hardware.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
This patch set is based on Felipe's usb.git / testing/next branch
(commit id = 47265c067c0d129f3a0e94bc221293a780af9d78).

Changes from v5 (https://patchwork.kernel.org/patch/10426483/):
 - Add a new function "usb3_set_mode_by_role_sw()" instead of reusing
   usb3_set_mode() and add _usb3_set_mode().
 - Change a coding style of container_of.
 - Revise an error message in renesas_usb3_role_switch_set().
 - Add "const" for "renesas_usb3_role_switch_desc".
 - Simplify a condition check of usb_of_get_companion_dev().

Changes from RFC v4 (https://patchwork.kernel.org/patch/10418265/):
 - Use "companion" device tree property simply instead of device_connection
   APIs with OF graph.
 - Merge patch 2 and 3 to one.
 - Revise the commit log (I should had revised this on RFC v4 though).

Changes from RFC v3:
 - Rebase latest usb.git / testing/next branch.
 - Add graph parse into device_connection_find_match().
 - Use workqueue to call _usb3_set_mode() in patch 3.
   (I realized renesas_usb3_role_switch_set() cannot run on atomic because
    device_attach() might sleep.)

Changes from RFC v2:
 - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
   because hardware resource (a register) is shared and remove individual
   usb role switch driver/dt-bindings for R-Car.
 - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
   doesn't need such API now.

Changes from RFC:
 - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
 - Remove drivers/of code.
 - Add a new API for find the connection by using graph on devcon.c and roles.c.
 - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
 - Update the dtsi file for r8a7795.


 drivers/usb/gadget/udc/Kconfig        |  1 +
 drivers/usb/gadget/udc/renesas_usb3.c | 84 ++++++++++++++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index b838cae..78823cd 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -193,6 +193,7 @@ config USB_RENESAS_USB3
 	tristate 'Renesas USB3.0 Peripheral controller'
 	depends on ARCH_RENESAS || COMPILE_TEST
 	depends on EXTCON && HAS_DMA
+	select USB_ROLE_SWITCH
 	help
 	   Renesas USB3.0 Peripheral controller is a USB peripheral controller
 	   that supports super, high, and full speed USB 3.0 data transfers.
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 5caf78b..f44a4b1 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -23,6 +23,8 @@
 #include <linux/uaccess.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb/of.h>
+#include <linux/usb/role.h>
 
 /* register definitions */
 #define USB3_AXI_INT_STA	0x008
@@ -335,6 +337,11 @@ struct renesas_usb3 {
 	struct phy *phy;
 	struct dentry *dentry;
 
+	struct usb_role_switch *role_sw;
+	struct device *host_dev;
+	struct work_struct role_work;
+	enum usb_role role;
+
 	struct renesas_usb3_ep *usb3_ep;
 	int num_usb3_eps;
 
@@ -651,6 +658,14 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
 	}
 }
 
+static void renesas_usb3_role_work(struct work_struct *work)
+{
+	struct renesas_usb3 *usb3 =
+			container_of(work, struct renesas_usb3, role_work);
+
+	usb_role_switch_set_role(usb3->role_sw, usb3->role);
+}
+
 static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
 	if (host)
@@ -659,6 +674,16 @@ static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 		usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
 }
 
+static void usb3_set_mode_by_role_sw(struct renesas_usb3 *usb3, bool host)
+{
+	if (usb3->role_sw) {
+		usb3->role = host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+		schedule_work(&usb3->role_work);
+	} else {
+		usb3_set_mode(usb3, host);
+	}
+}
+
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
 {
 	if (enable)
@@ -672,7 +697,7 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
 	unsigned long flags;
 
 	spin_lock_irqsave(&usb3->lock, flags);
-	usb3_set_mode(usb3, host);
+	usb3_set_mode_by_role_sw(usb3, host);
 	usb3_vbus_out(usb3, a_dev);
 	/* for A-Peripheral or forced B-device mode */
 	if ((!host && a_dev) ||
@@ -2302,6 +2327,41 @@ static int renesas_usb3_set_selfpowered(struct usb_gadget *gadget, int is_self)
 	.set_selfpowered	= renesas_usb3_set_selfpowered,
 };
 
+static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+	enum usb_role cur_role;
+
+	pm_runtime_get_sync(dev);
+	cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+	pm_runtime_put(dev);
+
+	return cur_role;
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+					enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+	struct device *host = usb3->host_dev;
+	enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+	pm_runtime_get_sync(dev);
+	if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
+		device_release_driver(host);
+		usb3_set_mode(usb3, false);
+	} else if (cur_role == USB_ROLE_DEVICE && role == USB_ROLE_HOST) {
+		/* Must set the mode before device_attach of the host */
+		usb3_set_mode(usb3, true);
+		/* This device_attach() might sleep */
+		if (device_attach(host) < 0)
+			dev_err(dev, "device_attach(host) failed\n");
+	}
+	pm_runtime_put(dev);
+
+	return 0;
+}
+
 static ssize_t role_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
@@ -2417,6 +2477,8 @@ static int renesas_usb3_remove(struct platform_device *pdev)
 	debugfs_remove_recursive(usb3->dentry);
 	device_remove_file(&pdev->dev, &dev_attr_role);
 
+	usb_role_switch_unregister(usb3->role_sw);
+
 	usb_del_gadget_udc(&usb3->gadget);
 	renesas_usb3_dma_free_prd(usb3, &pdev->dev);
 
@@ -2573,6 +2635,12 @@ static void renesas_usb3_init_ram(struct renesas_usb3 *usb3, struct device *dev,
 	EXTCON_NONE,
 };
 
+static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+	.set = renesas_usb3_role_switch_set,
+	.get = renesas_usb3_role_switch_get,
+	.allow_userspace_control = true,
+};
+
 static int renesas_usb3_probe(struct platform_device *pdev)
 {
 	struct renesas_usb3 *usb3;
@@ -2658,6 +2726,20 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
+	usb3->role_sw = usb_role_switch_register(&pdev->dev,
+					&renesas_usb3_role_switch_desc);
+	if (!IS_ERR(usb3->role_sw)) {
+		usb3->host_dev = usb_of_get_companion_dev(&pdev->dev);
+		if (!usb3->host_dev) {
+			/* If not found, this driver will not use a role sw */
+			usb_role_switch_unregister(usb3->role_sw);
+			usb3->role_sw = NULL;
+		}
+	} else {
+		usb3->role_sw = NULL;
+	}
+
 	usb3->workaround_for_vbus = priv->workaround_for_vbus;
 
 	renesas_usb3_debugfs_init(usb3, &pdev->dev);
-- 
1.9.1

^ permalink raw reply related

* [PATCHv8 3/3] ARM:drm ivip Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  6:01 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, linux-arm-kernel
In-Reply-To: <1527746514-3861-1-git-send-email-hean.loong.ong@intel.com>

From: Ong Hean Loong <hean.loong.ong@intel.com>

Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II.
The driver only supports the Intel Arria10 devkit and its variants.
This driver can be either loaded staticlly or in modules.
The OF device tree binding is located at:
Documentation/devicetree/bindings/display/altr,vip-fb2.txt

Signed-off-by: Ong Hean Loong <hean.loong.ong@intel.com>
---
 drivers/gpu/drm/Kconfig               |    2 +
 drivers/gpu/drm/Makefile              |    1 +
 drivers/gpu/drm/ivip/Kconfig          |   14 +++
 drivers/gpu/drm/ivip/Makefile         |    9 ++
 drivers/gpu/drm/ivip/intel_vip_conn.c |   95 ++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_core.c |  163 ++++++++++++++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h  |   52 +++++++++
 drivers/gpu/drm/ivip/intel_vip_of.c   |  193 +++++++++++++++++++++++++++++++++
 8 files changed, 529 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpu/drm/ivip/Kconfig
 create mode 100644 drivers/gpu/drm/ivip/Makefile
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7..cdc8e1a 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -204,6 +204,8 @@ source "drivers/gpu/drm/nouveau/Kconfig"
 
 source "drivers/gpu/drm/i915/Kconfig"
 
+source "drivers/gpu/drm/ivip/Kconfig"
+
 config DRM_VGEM
 	tristate "Virtual GEM provider"
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff..c0fba1d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_DRM_AMDGPU)+= amd/amdgpu/
 obj-$(CONFIG_DRM_MGA)	+= mga/
 obj-$(CONFIG_DRM_I810)	+= i810/
 obj-$(CONFIG_DRM_I915)	+= i915/
+obj-$(CONFIG_DRM_IVIP) += ivip/
 obj-$(CONFIG_DRM_MGAG200) += mgag200/
 obj-$(CONFIG_DRM_VC4)  += vc4/
 obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
diff --git a/drivers/gpu/drm/ivip/Kconfig b/drivers/gpu/drm/ivip/Kconfig
new file mode 100644
index 0000000..1d08b90
--- /dev/null
+++ b/drivers/gpu/drm/ivip/Kconfig
@@ -0,0 +1,14 @@
+config DRM_IVIP
+        tristate "Intel FGPA Video and Image Processing"
+        depends on DRM && OF
+        select DRM_GEM_CMA_HELPER
+        select DRM_KMS_HELPER
+        select DRM_KMS_FB_HELPER
+        select DRM_KMS_CMA_HELPER
+        help
+		  Choose this option if you have an Intel FPGA Arria 10 system
+		  and above with an Intel Display Port IP. This does not support
+		  legacy Intel FPGA Cyclone V display port. Currently only single
+		  frame buffer is supported. Note that ACPI and X_86 architecture
+		  is not supported for Arria10. If M is selected the module will be
+		  called ivip.
diff --git a/drivers/gpu/drm/ivip/Makefile b/drivers/gpu/drm/ivip/Makefile
new file mode 100644
index 0000000..cc55b04
--- /dev/null
+++ b/drivers/gpu/drm/ivip/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for the drm device driver.  This driver provides support for the
+# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+
+ccflags-y := -Iinclude/drm
+
+obj-$(CONFIG_DRM_IVIP) += ivip.o
+ivip-objs := intel_vip_of.o intel_vip_core.o \
+	intel_vip_conn.o
diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c
new file mode 100644
index 0000000..46bb04c
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_conn.c
@@ -0,0 +1,95 @@
+/*
+ * intel_vip_conn.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/drm_plane_helper.h>
+
+static enum drm_connector_status
+intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static void intelvipfb_drm_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = intelvipfb_drm_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = intelvipfb_drm_connector_destroy,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector)
+{
+	struct drm_device *drm = connector->dev;
+	int count;
+
+	count = drm_add_modes_noedid(connector, drm->mode_config.max_width,
+			drm->mode_config.max_height);
+	drm_set_preferred_mode(connector, drm->mode_config.max_width,
+			drm->mode_config.max_height);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+intelvipfb_drm_connector_helper_funcs = {
+	.get_modes = intelvipfb_drm_connector_get_modes,
+};
+
+struct drm_connector *
+intelvipfb_conn_setup(struct drm_device *drm)
+{
+	struct drm_connector *conn;
+	int ret;
+
+	conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL);
+	if (IS_ERR(conn))
+		return NULL;
+
+	ret = drm_connector_init(drm, conn, &intelvipfb_drm_connector_funcs,
+			DRM_MODE_CONNECTOR_DisplayPort);
+	if (ret < 0) {
+		dev_err(drm->dev, "failed to initialize drm connector\n");
+		ret = -ENOMEM;
+		goto error_connector_cleanup;
+	}
+
+	conn->polled = 0;
+	drm_connector_helper_add(conn, &intelvipfb_drm_connector_helper_funcs);
+
+	return conn;
+
+error_connector_cleanup:
+	drm_connector_cleanup(conn);
+
+	return NULL;
+}
diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c
new file mode 100644
index 0000000..a09e1ca
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_core.c
@@ -0,0 +1,163 @@
+/*
+ * intel_vip_core.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "intel_vip_drv.h"
+
+static void intelvipfb_enable(struct drm_simple_display_pipe *pipe,
+	       struct drm_crtc_state *crtc_state)
+{
+	/*
+	 * The frameinfo variable has to correspond to the size of the VIP Suite
+	 * Frame Reader register 7 which will determine the maximum size used
+	 * in this frameinfo
+	 */
+
+	u32 frameinfo;
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->base;
+	struct drm_plane_state *state = pipe->plane.state;
+	dma_addr_t addr;
+
+	addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
+
+	dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
+
+	frameinfo =
+		readl(base + INTELVIPFB_FRAME_READER) & 0x00ffffff;
+	writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
+	writel(addr, base + INTELVIPFB_FRAME_START);
+	/* Finally set the control register to 1 to start streaming */
+	writel(1, base + INTELVIPFB_CONTROL);
+}
+
+static void intelvipfb_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->base;
+	/* set the control register to 0 to stop streaming */
+	writel(0, base + INTELVIPFB_CONTROL);
+}
+
+static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static void intelvipfb_setup_mode_config(struct drm_device *drm)
+{
+	drm_mode_config_init(drm);
+	drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
+}
+
+static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
+					struct drm_plane_state *plane_state)
+{
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
+}
+
+
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
+	.prepare_fb = intelvipfb_pipe_prepare_fb,
+	.enable = intelvipfb_enable,
+	.disable = intelvipfb_disable
+};
+
+int intelvipfb_probe(struct device *dev)
+{
+	int retval;
+	struct drm_device *drm;
+	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
+	struct drm_connector *connector;
+	u32 formats[] = {DRM_FORMAT_XRGB8888};
+
+	drm = fbpriv->drm;
+
+	drm->dev_private = fbpriv;
+
+	intelvipfb_setup_mode_config(drm);
+
+	connector = intelvipfb_conn_setup(drm);
+	if (!connector) {
+		dev_err(drm->dev, "Connector setup failed\n");
+		goto err_mode_config;
+	}
+
+	retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe,
+			&fbpriv_funcs, formats,
+			ARRAY_SIZE(formats), NULL, connector);
+	if (retval < 0) {
+		dev_err(drm->dev, "Cannot setup simple display pipe\n");
+		goto err_mode_config;
+	}
+
+	fbpriv->fbcma = drm_fbdev_cma_init(drm,
+			drm->mode_config.preferred_depth,
+			drm->mode_config.num_connector);
+
+	drm_mode_config_reset(drm);
+
+	drm_dev_register(drm, 0);
+
+	return retval;
+
+err_mode_config:
+
+	drm_mode_config_cleanup(drm);
+	return -ENODEV;
+}
+
+int intelvipfb_remove(struct device *dev)
+{
+	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
+	struct drm_device *drm =  fbpriv->drm;
+
+	drm_dev_unregister(drm);
+
+	if (fbpriv->fbcma)
+		drm_fbdev_cma_fini(fbpriv->fbcma);
+
+	drm_mode_config_cleanup(drm);
+	drm_dev_unref(drm);
+
+	return 0;
+}
+
+MODULE_AUTHOR("Ong, Hean-Loong <hean.loong.ong@intel.com>");
+MODULE_DESCRIPTION("Intel VIP Frame Buffer II driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/ivip/intel_vip_drv.h b/drivers/gpu/drm/ivip/intel_vip_drv.h
new file mode 100644
index 0000000..0a3555d
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_drv.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2017 Intel Corporation.
+ *
+ * Intel Video and Image Processing(VIP) Frame Buffer II driver.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+#ifndef _INTEL_VIP_DRV_H
+#define _INTEL_VIP_DRV_H
+
+#define DRIVER_NAME    "intelvipfb"
+#define BYTES_PER_PIXEL	 4
+#define CRTC_NUM	        1
+#define CONN_NUM	        1
+
+/* control registers */
+#define INTELVIPFB_CONTROL	      0
+#define INTELVIPFB_STATUS	       0x4
+#define INTELVIPFB_INTERRUPT	    0x8
+#define INTELVIPFB_FRAME_COUNTER	0xC
+#define INTELVIPFB_FRAME_DROP	   0x10
+#define INTELVIPFB_FRAME_INFO	   0x14
+#define INTELVIPFB_FRAME_START	  0x18
+#define INTELVIPFB_FRAME_READER	         0x1C
+
+int intelvipfb_probe(struct device *dev);
+int intelvipfb_remove(struct device *dev);
+int intelvipfb_setup_crtc(struct drm_device *drm);
+struct drm_connector *intelvipfb_conn_setup(struct drm_device *drm);
+
+struct intelvipfb_priv {
+	struct drm_simple_display_pipe pipe;
+	struct drm_fbdev_cma *fbcma;
+	struct drm_device *drm;
+	void    __iomem *base;
+};
+
+#endif
diff --git a/drivers/gpu/drm/ivip/intel_vip_of.c b/drivers/gpu/drm/ivip/intel_vip_of.c
new file mode 100644
index 0000000..47302f9
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_of.c
@@ -0,0 +1,193 @@
+/*
+ * intel_vip_of.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#include <linux/component.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "intel_vip_drv.h"
+
+DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
+
+static void intelvipfb_lastclose(struct drm_device *drm)
+{
+	struct intelvipfb_priv *priv = drm->dev_private;
+
+	drm_fbdev_cma_restore_mode(priv->fbcma);
+}
+
+static struct drm_driver intelvipfb_drm = {
+	.driver_features =
+			DRIVER_MODESET | DRIVER_GEM |
+			DRIVER_PRIME | DRIVER_ATOMIC,
+	.gem_free_object_unlocked = drm_gem_cma_free_object,
+	.gem_vm_ops = &drm_gem_cma_vm_ops,
+	.dumb_create = drm_gem_cma_dumb_create,
+	.dumb_destroy = drm_gem_dumb_destroy,
+	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.gem_prime_export = drm_gem_prime_export,
+	.gem_prime_import = drm_gem_prime_import,
+	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_vmap = drm_gem_cma_prime_vmap,
+	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
+	.gem_prime_mmap = drm_gem_cma_prime_mmap,
+	.lastclose = intelvipfb_lastclose,
+	.name = DRIVER_NAME,
+	.date = "20170729",
+	.desc = "Intel FPGA VIP SUITE",
+	.major = 1,
+	.minor = 0,
+	.ioctls = NULL,
+	.patchlevel = 0,
+	.fops = &drm_fops,
+};
+
+/*
+ * Setting up information derived from OF Device Tree Nodes
+ * max-width, max-height, bits per pixel, memory port width
+ */
+
+static int intelvipfb_drm_setup(struct device *dev,
+					struct intelvipfb_priv *fbpriv)
+{
+	struct drm_device *drm = fbpriv->drm;
+	struct device_node *np = dev->of_node;
+	int mem_word_width;
+	int max_h, max_w;
+	int ret;
+
+	ret = of_property_read_u32(np, "altr,max-width", &max_w);
+	if (ret) {
+		dev_err(dev,
+			"Missing required parameter 'altr,max-width'");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "altr,max-height", &max_h);
+	if (ret) {
+		dev_err(dev,
+			"Missing required parameter 'altr,max-height'");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "altr,mem-port-width", &mem_word_width);
+	if (ret) {
+		dev_err(dev, "Missing required parameter 'altr,mem-port-width '");
+		return ret;
+	}
+
+	if (!(mem_word_width >= 32 && mem_word_width % 32 == 0)) {
+		dev_err(dev,
+			"mem-word-width is set to %i. must be >= 32 and multiple of 32.",
+			 mem_word_width);
+		return -ENODEV;
+	}
+
+	drm->mode_config.min_width = 640;
+	drm->mode_config.min_height = 480;
+	drm->mode_config.max_width = max_w;
+	drm->mode_config.max_height = max_h;
+	drm->mode_config.preferred_depth = 32;
+
+	return 0;
+}
+
+static int intelvipfb_of_probe(struct platform_device *pdev)
+{
+	int retval;
+	struct resource *reg_res;
+	struct intelvipfb_priv *fbpriv;
+	struct device *dev = &pdev->dev;
+	struct drm_device *drm;
+
+	fbpriv = devm_kzalloc(dev, sizeof(*fbpriv), GFP_KERNEL);
+	if (!fbpriv)
+		return -ENOMEM;
+
+	/*setup DRM */
+	drm = drm_dev_alloc(&intelvipfb_drm, dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	retval = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
+	if (retval)
+		return -ENODEV;
+
+	fbpriv->drm = drm;
+
+	reg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!reg_res)
+		return -ENOMEM;
+
+	fbpriv->base = devm_ioremap_resource(dev, reg_res);
+
+	if (IS_ERR(fbpriv->base)) {
+		dev_err(dev, "devm_ioremap_resource failed\n");
+		retval = PTR_ERR(fbpriv->base);
+		return -ENOMEM;
+	}
+
+	intelvipfb_drm_setup(dev, fbpriv);
+
+	dev_set_drvdata(dev, fbpriv);
+
+	return intelvipfb_probe(dev);
+}
+
+static int intelvipfb_of_remove(struct platform_device *pdev)
+{
+	return intelvipfb_remove(&pdev->dev);
+}
+
+/*
+ * The name vip-frame-buffer-2.0 is derived from
+ * http://www.altera.com/literature/ug/ug_vip.pdf
+ * frame buffer IP cores section 14
+ */
+
+static const struct of_device_id intelvipfb_of_match[] = {
+	{ .compatible = "altr,vip-frame-buffer-2.0" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, intelvipfb_of_match);
+
+static struct platform_driver intelvipfb_driver = {
+	.probe = intelvipfb_of_probe,
+	.remove = intelvipfb_of_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = intelvipfb_of_match,
+	},
+};
+
+module_platform_driver(intelvipfb_driver);
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCHv8 2/3] ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  6:01 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel
In-Reply-To: <1527746514-3861-1-git-send-email-hean.loong.ong@intel.com>

From: Ong Hean Loong <hean.loong.ong@intel.com>

Intel FPGA Video and Image Processing Suite Frame Buffer II
driver config for Arria 10 devkit and its variants

Signed-off-by: Ong, Hean Loong <hean.loong.ong@intel.com>
---
 arch/arm/configs/socfpga_defconfig |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 371fca4..21d8d2b 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -112,6 +112,11 @@ CONFIG_MFD_ALTERA_A10SR=y
 CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
+CONFIG_DRM=m
+CONFIG_DRM_IVIP=m
+CONFIG_DRM_IVIP_OF=m
+CONFIG_FB=y
+CONFIG_FB_SIMPLE=y
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_DWC2=y
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCHv8 1/3] ARM:dt-bindings:display Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  6:01 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel
In-Reply-To: <1527746514-3861-1-git-send-email-hean.loong.ong@intel.com>

From: Ong, Hean Loong <hean.loong.ong@intel.com>

Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel
Arria10 devkit and its variants. Vendor name retained as altr.

V8:
*Add port to Display port decoder

V7:
*Fix OF graph for better description
*Add description for encoder

V6:
*Description have not describe DT device in general

V5:
*remove bindings for bits per symbol as it has only one value which is 8

V4:
*fix properties that does not describe the values

V3:
*OF graph not in accordance to graph.txt

V2:
*Remove Linux driver description

V1:
*Missing vendor prefix

Signed-off-by: Ong, Hean Loong <hean.loong.ong@intel.com>
---
 .../devicetree/bindings/display/altr,vip-fb2.txt   |   99 ++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt

diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
new file mode 100644
index 0000000..4092804
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
@@ -0,0 +1,99 @@
+Intel Video and Image Processing(VIP) Frame Buffer II bindings
+
+Supported hardware: Intel FPGA SoC Arria10 and above with display port IP
+
+The Video Frame Buffer II in Video Image Processing (VIP) suite is an IP core
+that interfaces between system memory and Avalon-ST video ports. The IP core
+can be configured to support the memory reader (from memory to Avalon-ST)
+and/or memory writer (from Avalon-ST to memory) interfaces.
+
+More information the FPGA video IP component can be acquired from
+https://www.altera.com/content/dam/altera-www/global/en_US/pdfs\
+/literature/ug/ug_vip.pdf
+
+DT-Bindings:
+=============
+Required properties:
+----------------------------
+- compatible: "altr,vip-frame-buffer-2.0"
+- reg: Physical base address and length of the framebuffer controller's
+	registers.
+- altr,max-width: The maximum width of the framebuffer in pixels.
+- altr,max-height: The maximum height of the framebuffer in pixels.
+- altr,mem-port-width = the bus width of the avalon master port
+	on the frame reader
+
+Optional sub-nodes:
+- ports: The connection to the encoder
+
+Optional properties
+----------------------------
+- compatible: "altr, display-port"
+- reg: Physical base address and length of the display port controller's
+	registers
+- clocks: required clock handles for specified pairs in clock name
+- clock-names: required clock names. Contains:
+	- aux_clk: auxiliary clock,
+	- clk: 100 MHz output clock
+	- xcvr_mgmt_clk: transceiver management clock
+
+Optional sub-nodes:
+- ports: The connection to the controller
+
+Connections between the Frame Buffer II and other video IP cores in the system
+are modelled using the OF graph DT bindings. The Frame Buffer II node has up
+to two OF graph ports. When the memory writer interface is enabled, port 0
+maps to the Avalon-ST Input (din) port. When the memory reader interface is
+enabled, port 1 maps to the Avalon-ST Output (dout) port.
+
+The encoder is built into the FPGA HW design and therefore would not
+be accessible from the DDR.
+
+		Port 0				Port1
+---------------------------------------------------------
+ARRIA10 AVALON_ST (DIN)		AVALON_ST (DOUT)
+
+Required Properties Example:
+----------------------------
+
+framebuffer@100000280 {
+		compatible = "altr,vip-frame-buffer-2.0";
+		reg = <0x00000001 0x00000280 0x00000040>;
+		altr,max-width = <1280>;
+		altr,max-height = <720>;
+		altr,mem-port-width = <128>;
+
+		ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+			port@1 {
+				reg = <1>;
+					fb_output: endpoint {
+						remote-endpoint = <&dp_encoder_input>;
+					};
+			};
+		};
+};
+
+Optional Properties Example:
+This is not required unless there are needs to customize
+Display Port controller settings.
+
+displayport@100002000 {
+		compatible = "altr, display-port";
+		reg = <0x00000001 0x00002000 0x00000800>;
+		clocks = <&dp_0_clk_16 &dp_0_clk_100 &dp_0_clk_100>;
+		clock-names = "aux_clk", "clk", "xcvr_mgmt_clk";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <1>;
+					dp_input: endpoint {
+						remote-endpoint = <&dp_controller_input>;
+					};
+			};
+};
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCHv8 0/3] Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  6:01 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel

From: Ong, Hean Loong <hean.loong.ong@intel.com>

The FPGA FrameBuffer Soft IP could be seen  as the GPU and the DRM driver patch here is allocating memory for information to be streamed from the ARM/Linux to the display port.
Basically the driver just wraps the information such as the pixels to be drawn by the FPGA FrameBuffer 2.

The piece of hardware in discussion is the SoC FPGA where Linux runs on the ARM chip and the FGPA is driven by its NIOS soft core with its own proprietary firmware.

For example the application from the ARM Linux would have to write information on the /dev/fb0 with the information stored in the SDRAM to be fetched by the FPGA framebuffer IP and displayed on the Display Port Monitor.

Ong Hean Loong (3):
  ARM:dt-bindings:display Intel FPGA Video and Image Processing Suite
  ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite
  ARM:drm ivip Intel FPGA Video and Image Processing Suite

 .../devicetree/bindings/display/altr,vip-fb2.txt   |  112 +++++++++++
 arch/arm/configs/socfpga_defconfig                 |    5 +
 drivers/gpu/drm/Kconfig                            |    2 +
 drivers/gpu/drm/Makefile                           |    1 +
 drivers/gpu/drm/ivip/Kconfig                       |   14 ++
 drivers/gpu/drm/ivip/Makefile                      |    9 +
 drivers/gpu/drm/ivip/intel_vip_conn.c              |   96 ++++++++++
 drivers/gpu/drm/ivip/intel_vip_core.c              |  162 ++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h               |   52 ++++++
 drivers/gpu/drm/ivip/intel_vip_of.c                |  193 ++++++++++++++++++++
 10 files changed, 646 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
 create mode 100644 drivers/gpu/drm/ivip/Kconfig
 create mode 100644 drivers/gpu/drm/ivip/Makefile
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* [PATCH] ARM:drm ivip Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  5:50 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, linux-arm-kernel
In-Reply-To: <1527745851-3339-1-git-send-email-hean.loong.ong@intel.com>

From: Ong Hean Loong <hean.loong.ong@intel.com>

Driver for Intel FPGA Video and Image Processing Suite Frame Buffer II.
The driver only supports the Intel Arria10 devkit and its variants.
This driver can be either loaded staticlly or in modules.
The OF device tree binding is located at:
Documentation/devicetree/bindings/display/altr,vip-fb2.txt

Signed-off-by: Ong Hean Loong <hean.loong.ong@intel.com>
---
 drivers/gpu/drm/Kconfig               |    2 +
 drivers/gpu/drm/Makefile              |    1 +
 drivers/gpu/drm/ivip/Kconfig          |   14 +++
 drivers/gpu/drm/ivip/Makefile         |    9 ++
 drivers/gpu/drm/ivip/intel_vip_conn.c |   95 ++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_core.c |  163 ++++++++++++++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h  |   52 +++++++++
 drivers/gpu/drm/ivip/intel_vip_of.c   |  193 +++++++++++++++++++++++++++++++++
 8 files changed, 529 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpu/drm/ivip/Kconfig
 create mode 100644 drivers/gpu/drm/ivip/Makefile
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index deeefa7..cdc8e1a 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -204,6 +204,8 @@ source "drivers/gpu/drm/nouveau/Kconfig"
 
 source "drivers/gpu/drm/i915/Kconfig"
 
+source "drivers/gpu/drm/ivip/Kconfig"
+
 config DRM_VGEM
 	tristate "Virtual GEM provider"
 	depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 50093ff..c0fba1d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_DRM_AMDGPU)+= amd/amdgpu/
 obj-$(CONFIG_DRM_MGA)	+= mga/
 obj-$(CONFIG_DRM_I810)	+= i810/
 obj-$(CONFIG_DRM_I915)	+= i915/
+obj-$(CONFIG_DRM_IVIP) += ivip/
 obj-$(CONFIG_DRM_MGAG200) += mgag200/
 obj-$(CONFIG_DRM_VC4)  += vc4/
 obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus/
diff --git a/drivers/gpu/drm/ivip/Kconfig b/drivers/gpu/drm/ivip/Kconfig
new file mode 100644
index 0000000..1d08b90
--- /dev/null
+++ b/drivers/gpu/drm/ivip/Kconfig
@@ -0,0 +1,14 @@
+config DRM_IVIP
+        tristate "Intel FGPA Video and Image Processing"
+        depends on DRM && OF
+        select DRM_GEM_CMA_HELPER
+        select DRM_KMS_HELPER
+        select DRM_KMS_FB_HELPER
+        select DRM_KMS_CMA_HELPER
+        help
+		  Choose this option if you have an Intel FPGA Arria 10 system
+		  and above with an Intel Display Port IP. This does not support
+		  legacy Intel FPGA Cyclone V display port. Currently only single
+		  frame buffer is supported. Note that ACPI and X_86 architecture
+		  is not supported for Arria10. If M is selected the module will be
+		  called ivip.
diff --git a/drivers/gpu/drm/ivip/Makefile b/drivers/gpu/drm/ivip/Makefile
new file mode 100644
index 0000000..cc55b04
--- /dev/null
+++ b/drivers/gpu/drm/ivip/Makefile
@@ -0,0 +1,9 @@
+#
+# Makefile for the drm device driver.  This driver provides support for the
+# Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
+
+ccflags-y := -Iinclude/drm
+
+obj-$(CONFIG_DRM_IVIP) += ivip.o
+ivip-objs := intel_vip_of.o intel_vip_core.o \
+	intel_vip_conn.o
diff --git a/drivers/gpu/drm/ivip/intel_vip_conn.c b/drivers/gpu/drm/ivip/intel_vip_conn.c
new file mode 100644
index 0000000..46bb04c
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_conn.c
@@ -0,0 +1,95 @@
+/*
+ * intel_vip_conn.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_encoder_slave.h>
+#include <drm/drm_plane_helper.h>
+
+static enum drm_connector_status
+intelvipfb_drm_connector_detect(struct drm_connector *connector, bool force)
+{
+	return connector_status_connected;
+}
+
+static void intelvipfb_drm_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs intelvipfb_drm_connector_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.detect = intelvipfb_drm_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = intelvipfb_drm_connector_destroy,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int intelvipfb_drm_connector_get_modes(struct drm_connector *connector)
+{
+	struct drm_device *drm = connector->dev;
+	int count;
+
+	count = drm_add_modes_noedid(connector, drm->mode_config.max_width,
+			drm->mode_config.max_height);
+	drm_set_preferred_mode(connector, drm->mode_config.max_width,
+			drm->mode_config.max_height);
+	return count;
+}
+
+static const struct drm_connector_helper_funcs
+intelvipfb_drm_connector_helper_funcs = {
+	.get_modes = intelvipfb_drm_connector_get_modes,
+};
+
+struct drm_connector *
+intelvipfb_conn_setup(struct drm_device *drm)
+{
+	struct drm_connector *conn;
+	int ret;
+
+	conn = devm_kzalloc(drm->dev, sizeof(*conn), GFP_KERNEL);
+	if (IS_ERR(conn))
+		return NULL;
+
+	ret = drm_connector_init(drm, conn, &intelvipfb_drm_connector_funcs,
+			DRM_MODE_CONNECTOR_DisplayPort);
+	if (ret < 0) {
+		dev_err(drm->dev, "failed to initialize drm connector\n");
+		ret = -ENOMEM;
+		goto error_connector_cleanup;
+	}
+
+	conn->polled = 0;
+	drm_connector_helper_add(conn, &intelvipfb_drm_connector_helper_funcs);
+
+	return conn;
+
+error_connector_cleanup:
+	drm_connector_cleanup(conn);
+
+	return NULL;
+}
diff --git a/drivers/gpu/drm/ivip/intel_vip_core.c b/drivers/gpu/drm/ivip/intel_vip_core.c
new file mode 100644
index 0000000..a09e1ca
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_core.c
@@ -0,0 +1,163 @@
+/*
+ * intel_vip_core.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "intel_vip_drv.h"
+
+static void intelvipfb_enable(struct drm_simple_display_pipe *pipe,
+	       struct drm_crtc_state *crtc_state)
+{
+	/*
+	 * The frameinfo variable has to correspond to the size of the VIP Suite
+	 * Frame Reader register 7 which will determine the maximum size used
+	 * in this frameinfo
+	 */
+
+	u32 frameinfo;
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->base;
+	struct drm_plane_state *state = pipe->plane.state;
+	dma_addr_t addr;
+
+	addr = drm_fb_cma_get_gem_addr(state->fb, state, 0);
+
+	dev_info(pipe->plane.dev->dev, "Address 0x%x\n", addr);
+
+	frameinfo =
+		readl(base + INTELVIPFB_FRAME_READER) & 0x00ffffff;
+	writel(frameinfo, base + INTELVIPFB_FRAME_INFO);
+	writel(addr, base + INTELVIPFB_FRAME_START);
+	/* Finally set the control register to 1 to start streaming */
+	writel(1, base + INTELVIPFB_CONTROL);
+}
+
+static void intelvipfb_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct intelvipfb_priv *priv = pipe->plane.dev->dev_private;
+	void __iomem *base = priv->base;
+	/* set the control register to 0 to stop streaming */
+	writel(0, base + INTELVIPFB_CONTROL);
+}
+
+static const struct drm_mode_config_funcs intelvipfb_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static void intelvipfb_setup_mode_config(struct drm_device *drm)
+{
+	drm_mode_config_init(drm);
+	drm->mode_config.funcs = &intelvipfb_mode_config_funcs;
+}
+
+static int intelvipfb_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
+					struct drm_plane_state *plane_state)
+{
+	return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
+}
+
+
+static struct drm_simple_display_pipe_funcs fbpriv_funcs = {
+	.prepare_fb = intelvipfb_pipe_prepare_fb,
+	.enable = intelvipfb_enable,
+	.disable = intelvipfb_disable
+};
+
+int intelvipfb_probe(struct device *dev)
+{
+	int retval;
+	struct drm_device *drm;
+	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
+	struct drm_connector *connector;
+	u32 formats[] = {DRM_FORMAT_XRGB8888};
+
+	drm = fbpriv->drm;
+
+	drm->dev_private = fbpriv;
+
+	intelvipfb_setup_mode_config(drm);
+
+	connector = intelvipfb_conn_setup(drm);
+	if (!connector) {
+		dev_err(drm->dev, "Connector setup failed\n");
+		goto err_mode_config;
+	}
+
+	retval = drm_simple_display_pipe_init(drm, &fbpriv->pipe,
+			&fbpriv_funcs, formats,
+			ARRAY_SIZE(formats), NULL, connector);
+	if (retval < 0) {
+		dev_err(drm->dev, "Cannot setup simple display pipe\n");
+		goto err_mode_config;
+	}
+
+	fbpriv->fbcma = drm_fbdev_cma_init(drm,
+			drm->mode_config.preferred_depth,
+			drm->mode_config.num_connector);
+
+	drm_mode_config_reset(drm);
+
+	drm_dev_register(drm, 0);
+
+	return retval;
+
+err_mode_config:
+
+	drm_mode_config_cleanup(drm);
+	return -ENODEV;
+}
+
+int intelvipfb_remove(struct device *dev)
+{
+	struct intelvipfb_priv *fbpriv = dev_get_drvdata(dev);
+	struct drm_device *drm =  fbpriv->drm;
+
+	drm_dev_unregister(drm);
+
+	if (fbpriv->fbcma)
+		drm_fbdev_cma_fini(fbpriv->fbcma);
+
+	drm_mode_config_cleanup(drm);
+	drm_dev_unref(drm);
+
+	return 0;
+}
+
+MODULE_AUTHOR("Ong, Hean-Loong <hean.loong.ong@intel.com>");
+MODULE_DESCRIPTION("Intel VIP Frame Buffer II driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/gpu/drm/ivip/intel_vip_drv.h b/drivers/gpu/drm/ivip/intel_vip_drv.h
new file mode 100644
index 0000000..0a3555d
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_drv.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2017 Intel Corporation.
+ *
+ * Intel Video and Image Processing(VIP) Frame Buffer II driver.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+#ifndef _INTEL_VIP_DRV_H
+#define _INTEL_VIP_DRV_H
+
+#define DRIVER_NAME    "intelvipfb"
+#define BYTES_PER_PIXEL	 4
+#define CRTC_NUM	        1
+#define CONN_NUM	        1
+
+/* control registers */
+#define INTELVIPFB_CONTROL	      0
+#define INTELVIPFB_STATUS	       0x4
+#define INTELVIPFB_INTERRUPT	    0x8
+#define INTELVIPFB_FRAME_COUNTER	0xC
+#define INTELVIPFB_FRAME_DROP	   0x10
+#define INTELVIPFB_FRAME_INFO	   0x14
+#define INTELVIPFB_FRAME_START	  0x18
+#define INTELVIPFB_FRAME_READER	         0x1C
+
+int intelvipfb_probe(struct device *dev);
+int intelvipfb_remove(struct device *dev);
+int intelvipfb_setup_crtc(struct drm_device *drm);
+struct drm_connector *intelvipfb_conn_setup(struct drm_device *drm);
+
+struct intelvipfb_priv {
+	struct drm_simple_display_pipe pipe;
+	struct drm_fbdev_cma *fbcma;
+	struct drm_device *drm;
+	void    __iomem *base;
+};
+
+#endif
diff --git a/drivers/gpu/drm/ivip/intel_vip_of.c b/drivers/gpu/drm/ivip/intel_vip_of.c
new file mode 100644
index 0000000..47302f9
--- /dev/null
+++ b/drivers/gpu/drm/ivip/intel_vip_of.c
@@ -0,0 +1,193 @@
+/*
+ * intel_vip_of.c -- Intel Video and Image Processing(VIP)
+ * Frame Buffer II driver
+ *
+ * This driver supports the Intel VIP Frame Reader component.
+ * More info on the hardware can be found in the Intel Video
+ * and Image Processing Suite User Guide at this address
+ * http://www.altera.com/literature/ug/ug_vip.pdf.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * Authors:
+ * Ong, Hean-Loong <hean.loong.ong@intel.com>
+ *
+ */
+#include <drm/drm_fb_helper.h>
+#include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#include <linux/component.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#include "intel_vip_drv.h"
+
+DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
+
+static void intelvipfb_lastclose(struct drm_device *drm)
+{
+	struct intelvipfb_priv *priv = drm->dev_private;
+
+	drm_fbdev_cma_restore_mode(priv->fbcma);
+}
+
+static struct drm_driver intelvipfb_drm = {
+	.driver_features =
+			DRIVER_MODESET | DRIVER_GEM |
+			DRIVER_PRIME | DRIVER_ATOMIC,
+	.gem_free_object_unlocked = drm_gem_cma_free_object,
+	.gem_vm_ops = &drm_gem_cma_vm_ops,
+	.dumb_create = drm_gem_cma_dumb_create,
+	.dumb_destroy = drm_gem_dumb_destroy,
+	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
+	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
+	.gem_prime_export = drm_gem_prime_export,
+	.gem_prime_import = drm_gem_prime_import,
+	.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
+	.gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+	.gem_prime_vmap = drm_gem_cma_prime_vmap,
+	.gem_prime_vunmap = drm_gem_cma_prime_vunmap,
+	.gem_prime_mmap = drm_gem_cma_prime_mmap,
+	.lastclose = intelvipfb_lastclose,
+	.name = DRIVER_NAME,
+	.date = "20170729",
+	.desc = "Intel FPGA VIP SUITE",
+	.major = 1,
+	.minor = 0,
+	.ioctls = NULL,
+	.patchlevel = 0,
+	.fops = &drm_fops,
+};
+
+/*
+ * Setting up information derived from OF Device Tree Nodes
+ * max-width, max-height, bits per pixel, memory port width
+ */
+
+static int intelvipfb_drm_setup(struct device *dev,
+					struct intelvipfb_priv *fbpriv)
+{
+	struct drm_device *drm = fbpriv->drm;
+	struct device_node *np = dev->of_node;
+	int mem_word_width;
+	int max_h, max_w;
+	int ret;
+
+	ret = of_property_read_u32(np, "altr,max-width", &max_w);
+	if (ret) {
+		dev_err(dev,
+			"Missing required parameter 'altr,max-width'");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "altr,max-height", &max_h);
+	if (ret) {
+		dev_err(dev,
+			"Missing required parameter 'altr,max-height'");
+		return ret;
+	}
+
+	ret = of_property_read_u32(np, "altr,mem-port-width", &mem_word_width);
+	if (ret) {
+		dev_err(dev, "Missing required parameter 'altr,mem-port-width '");
+		return ret;
+	}
+
+	if (!(mem_word_width >= 32 && mem_word_width % 32 == 0)) {
+		dev_err(dev,
+			"mem-word-width is set to %i. must be >= 32 and multiple of 32.",
+			 mem_word_width);
+		return -ENODEV;
+	}
+
+	drm->mode_config.min_width = 640;
+	drm->mode_config.min_height = 480;
+	drm->mode_config.max_width = max_w;
+	drm->mode_config.max_height = max_h;
+	drm->mode_config.preferred_depth = 32;
+
+	return 0;
+}
+
+static int intelvipfb_of_probe(struct platform_device *pdev)
+{
+	int retval;
+	struct resource *reg_res;
+	struct intelvipfb_priv *fbpriv;
+	struct device *dev = &pdev->dev;
+	struct drm_device *drm;
+
+	fbpriv = devm_kzalloc(dev, sizeof(*fbpriv), GFP_KERNEL);
+	if (!fbpriv)
+		return -ENOMEM;
+
+	/*setup DRM */
+	drm = drm_dev_alloc(&intelvipfb_drm, dev);
+	if (IS_ERR(drm))
+		return PTR_ERR(drm);
+
+	retval = dma_set_mask_and_coherent(drm->dev, DMA_BIT_MASK(32));
+	if (retval)
+		return -ENODEV;
+
+	fbpriv->drm = drm;
+
+	reg_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!reg_res)
+		return -ENOMEM;
+
+	fbpriv->base = devm_ioremap_resource(dev, reg_res);
+
+	if (IS_ERR(fbpriv->base)) {
+		dev_err(dev, "devm_ioremap_resource failed\n");
+		retval = PTR_ERR(fbpriv->base);
+		return -ENOMEM;
+	}
+
+	intelvipfb_drm_setup(dev, fbpriv);
+
+	dev_set_drvdata(dev, fbpriv);
+
+	return intelvipfb_probe(dev);
+}
+
+static int intelvipfb_of_remove(struct platform_device *pdev)
+{
+	return intelvipfb_remove(&pdev->dev);
+}
+
+/*
+ * The name vip-frame-buffer-2.0 is derived from
+ * http://www.altera.com/literature/ug/ug_vip.pdf
+ * frame buffer IP cores section 14
+ */
+
+static const struct of_device_id intelvipfb_of_match[] = {
+	{ .compatible = "altr,vip-frame-buffer-2.0" },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, intelvipfb_of_match);
+
+static struct platform_driver intelvipfb_driver = {
+	.probe = intelvipfb_of_probe,
+	.remove = intelvipfb_of_remove,
+	.driver = {
+		.name = DRIVER_NAME,
+		.of_match_table = intelvipfb_of_match,
+	},
+};
+
+module_platform_driver(intelvipfb_driver);
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCH 2/3] ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  5:50 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel
In-Reply-To: <1527745851-3339-1-git-send-email-hean.loong.ong@intel.com>

From: Ong Hean Loong <hean.loong.ong@intel.com>

Intel FPGA Video and Image Processing Suite Frame Buffer II
driver config for Arria 10 devkit and its variants

Signed-off-by: Ong, Hean Loong <hean.loong.ong@intel.com>
---
 arch/arm/configs/socfpga_defconfig |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 2620ce7..d7deee8 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -111,6 +111,11 @@ CONFIG_MFD_ALTERA_A10SR=y
 CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
+CONFIG_DRM=m
+CONFIG_DRM_IVIP=m
+CONFIG_DRM_IVIP_OF=m
+CONFIG_FB=y
+CONFIG_FB_SIMPLE=y
 CONFIG_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_DWC2=y
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCH 1/3] ARM:dt-bindings:display Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  5:50 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel
In-Reply-To: <1527745851-3339-1-git-send-email-hean.loong.ong@intel.com>

From: Ong, Hean Loong <hean.loong.ong@intel.com>

Device tree binding for Intel FPGA Video and Image Processing Suite. The binding involved would be generated from the Altera (Intel) Qsys system. The bindings would set the max width, max height, buts per pixel and memory port width. The device tree binding only supports the Intel
Arria10 devkit and its variants. Vendor name retained as altr.

V8:
*Add port to Display port decoder

V7:
*Fix OF graph for better description
*Add description for encoder

V6:
*Description have not describe DT device in general

V5:
*remove bindings for bits per symbol as it has only one value which is 8

V4:
*fix properties that does not describe the values

V3:
*OF graph not in accordance to graph.txt

V2:
*Remove Linux driver description

V1:
*Missing vendor prefix

Signed-off-by: Ong, Hean Loong <hean.loong.ong@intel.com>
---
 .../devicetree/bindings/display/altr,vip-fb2.txt   |   99 ++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt

diff --git a/Documentation/devicetree/bindings/display/altr,vip-fb2.txt b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
new file mode 100644
index 0000000..4092804
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/altr,vip-fb2.txt
@@ -0,0 +1,99 @@
+Intel Video and Image Processing(VIP) Frame Buffer II bindings
+
+Supported hardware: Intel FPGA SoC Arria10 and above with display port IP
+
+The Video Frame Buffer II in Video Image Processing (VIP) suite is an IP core
+that interfaces between system memory and Avalon-ST video ports. The IP core
+can be configured to support the memory reader (from memory to Avalon-ST)
+and/or memory writer (from Avalon-ST to memory) interfaces.
+
+More information the FPGA video IP component can be acquired from
+https://www.altera.com/content/dam/altera-www/global/en_US/pdfs\
+/literature/ug/ug_vip.pdf
+
+DT-Bindings:
+=============
+Required properties:
+----------------------------
+- compatible: "altr,vip-frame-buffer-2.0"
+- reg: Physical base address and length of the framebuffer controller's
+	registers.
+- altr,max-width: The maximum width of the framebuffer in pixels.
+- altr,max-height: The maximum height of the framebuffer in pixels.
+- altr,mem-port-width = the bus width of the avalon master port
+	on the frame reader
+
+Optional sub-nodes:
+- ports: The connection to the encoder
+
+Optional properties
+----------------------------
+- compatible: "altr, display-port"
+- reg: Physical base address and length of the display port controller's
+	registers
+- clocks: required clock handles for specified pairs in clock name
+- clock-names: required clock names. Contains:
+	- aux_clk: auxiliary clock,
+	- clk: 100 MHz output clock
+	- xcvr_mgmt_clk: transceiver management clock
+
+Optional sub-nodes:
+- ports: The connection to the controller
+
+Connections between the Frame Buffer II and other video IP cores in the system
+are modelled using the OF graph DT bindings. The Frame Buffer II node has up
+to two OF graph ports. When the memory writer interface is enabled, port 0
+maps to the Avalon-ST Input (din) port. When the memory reader interface is
+enabled, port 1 maps to the Avalon-ST Output (dout) port.
+
+The encoder is built into the FPGA HW design and therefore would not
+be accessible from the DDR.
+
+		Port 0				Port1
+---------------------------------------------------------
+ARRIA10 AVALON_ST (DIN)		AVALON_ST (DOUT)
+
+Required Properties Example:
+----------------------------
+
+framebuffer@100000280 {
+		compatible = "altr,vip-frame-buffer-2.0";
+		reg = <0x00000001 0x00000280 0x00000040>;
+		altr,max-width = <1280>;
+		altr,max-height = <720>;
+		altr,mem-port-width = <128>;
+
+		ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+			port@1 {
+				reg = <1>;
+					fb_output: endpoint {
+						remote-endpoint = <&dp_encoder_input>;
+					};
+			};
+		};
+};
+
+Optional Properties Example:
+This is not required unless there are needs to customize
+Display Port controller settings.
+
+displayport@100002000 {
+		compatible = "altr, display-port";
+		reg = <0x00000001 0x00002000 0x00000800>;
+		clocks = <&dp_0_clk_16 &dp_0_clk_100 &dp_0_clk_100>;
+		clock-names = "aux_clk", "clk", "xcvr_mgmt_clk";
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <1>;
+					dp_input: endpoint {
+						remote-endpoint = <&dp_controller_input>;
+					};
+			};
+};
-- 
1.7.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related

* [PATCHv<8> 0/3] Intel FPGA Video and Image Processing Suite
From: Hean-Loong, Ong @ 2018-05-31  5:50 UTC (permalink / raw)
  To: Rob Herring, Dinh Nguyen, Daniel Vetter, Laurent Pinchart
  Cc: devicetree, yves.vandervennet, hean.loong.ong, chin.liang.see,
	linux-kernel, dri-devel, Ong, linux-arm-kernel

From: Ong, Hean Loong <hean.loong.ong@intel.com>

The FPGA FrameBuffer Soft IP could be seen  as the GPU and the DRM driver patch here is allocating memory for information to be streamed from the ARM/Linux to the display port.
Basically the driver just wraps the information such as the pixels to be drawn by the FPGA FrameBuffer 2.

The piece of hardware in discussion is the SoC FPGA where Linux runs on the ARM chip and the FGPA is driven by its NIOS soft core with its own proprietary firmware.

For example the application from the ARM Linux would have to write information on the /dev/fb0 with the information stored in the SDRAM to be fetched by the FPGA framebuffer IP and displayed on the Display Port Monitor.

Ong Hean Loong (3):
  ARM:dt-bindings:display Intel FPGA Video and Image Processing Suite
  ARM:socfpga-defconfig Intel FPGA Video and Image Processing Suite
  ARM:drm ivip Intel FPGA Video and Image Processing Suite

 .../devicetree/bindings/display/altr,vip-fb2.txt   |  112 +++++++++++
 arch/arm/configs/socfpga_defconfig                 |    5 +
 drivers/gpu/drm/Kconfig                            |    2 +
 drivers/gpu/drm/Makefile                           |    1 +
 drivers/gpu/drm/ivip/Kconfig                       |   14 ++
 drivers/gpu/drm/ivip/Makefile                      |    9 +
 drivers/gpu/drm/ivip/intel_vip_conn.c              |   96 ++++++++++
 drivers/gpu/drm/ivip/intel_vip_core.c              |  162 ++++++++++++++++
 drivers/gpu/drm/ivip/intel_vip_drv.h               |   52 ++++++
 drivers/gpu/drm/ivip/intel_vip_of.c                |  193 ++++++++++++++++++++
 10 files changed, 646 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/altr,vip-fb2.txt
 create mode 100644 drivers/gpu/drm/ivip/Kconfig
 create mode 100644 drivers/gpu/drm/ivip/Makefile
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_conn.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_core.c
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_drv.h
 create mode 100644 drivers/gpu/drm/ivip/intel_vip_of.c

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v2 2/2] clk: Add driver for MAX9485
From: Daniel Mack @ 2018-05-31  5:28 UTC (permalink / raw)
  To: mturquette, sboyd; +Cc: linux-clk, robh, devicetree, Daniel Mack
In-Reply-To: <20180525182058.25969-3-daniel@zonque.org>

On Friday, May 25, 2018 08:20 PM, Daniel Mack wrote:
> From: Daniel Mack <zonque@gmail.com>
> 
> This patch adds a driver for MAX9485, a programmable audio clock generator.
> 
> The device requires a 27.000 MHz clock input. It can provide a gated
> buffered output of its input clock and two gated outputs of a PLL that can
> generate one out of 16 discrete frequencies. There is only one PLL however,
> so the two gated outputs will always have the same frequency but they can
> be switched individually.
> 
> The driver for this device exposes 4 clocks in total:
> 
> - MAX9485_MCLKOUT:      A gated, buffered output of the input clock
> - MAX9485_CLKOUT:       A PLL that can be configured to 16 different
> 			discrete frequencies
> - MAX9485_CLKOUT[1,2]:  Two gated outputs for MAX9485_CLKOUT
> 
> Some PLL output frequencies can be achieved with different register
> settings. The driver will select the one with lowest jitter in such cases.
> 
> Signed-off-by: Daniel Mack <daniel@zonque.org>

Note that I've spotted a minor issue with the reset GPIO line handling 
that sneaked in in v2. I just didn't send v3 yet because I wanted to 
wait for more feedback on the implementation.


Thanks,
Daniel



> ---
>   drivers/clk/Kconfig       |   8 +
>   drivers/clk/Makefile      |   1 +
>   drivers/clk/clk-max9485.c | 408 ++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 417 insertions(+)
>   create mode 100644 drivers/clk/clk-max9485.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 34968a381d0f..f8ab54c41d16 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -45,6 +45,14 @@ config COMMON_CLK_MAX77686
>   	  This driver supports Maxim 77620/77686/77802 crystal oscillator
>   	  clock.
>   
> +config COMMON_CLK_MAX9485
> +	tristate "Clock driver for Maxim 9485 Programmable Clock Generator"
> +	depends on I2C
> +	depends on OF
> +	depends on GPIOLIB || COMPILE_TEST
> +	---help---
> +	  This driver supports Maxim 9485 Programmable Audio Clock Generator
> +
>   config COMMON_CLK_RK808
>   	tristate "Clock driver for RK805/RK808/RK818"
>   	depends on MFD_RK808
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index de6d06ac790b..27417ba3c1df 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -31,6 +31,7 @@ obj-$(CONFIG_COMMON_CLK_ASPEED)		+= clk-aspeed.o
>   obj-$(CONFIG_ARCH_HIGHBANK)		+= clk-highbank.o
>   obj-$(CONFIG_CLK_HSDK)			+= clk-hsdk-pll.o
>   obj-$(CONFIG_COMMON_CLK_MAX77686)	+= clk-max77686.o
> +obj-$(CONFIG_COMMON_CLK_MAX9485)	+= clk-max9485.o
>   obj-$(CONFIG_ARCH_MOXART)		+= clk-moxart.o
>   obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
>   obj-$(CONFIG_ARCH_NSPIRE)		+= clk-nspire.o
> diff --git a/drivers/clk/clk-max9485.c b/drivers/clk/clk-max9485.c
> new file mode 100644
> index 000000000000..9bbf2fee49c5
> --- /dev/null
> +++ b/drivers/clk/clk-max9485.c
> @@ -0,0 +1,408 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * clk-max9485.c: MAX9485 Programmable Audio Clock Generator
> + *
> + * (c) 2018 Daniel Mack <daniel@zonque.org>
> + *
> + * References:
> + *   MAX9485 Datasheet
> + *     http://www.maximintegrated.com/datasheet/index.mvp/id/4421
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <dt-bindings/clock/maxim,max9485.h>
> +
> +#define MAX9485_INPUT_FREQ 27000000UL
> +#define MAX9485_NUM_CLKS 4
> +
> +/* This chip has only one register of 8 bit width. */
> +
> +enum {
> +	MAX9485_FS_12KHZ	= 0 << 0,
> +	MAX9485_FS_32KHZ	= 1 << 0,
> +	MAX9485_FS_44_1KHZ	= 2 << 0,
> +	MAX9485_FS_48KHZ	= 3 << 0,
> +};
> +
> +enum {
> +	MAX9485_SCALE_256	= 0 << 2,
> +	MAX9485_SCALE_384	= 1 << 2,
> +	MAX9485_SCALE_768	= 2 << 2,
> +};
> +
> +#define MAX9485_DOUBLE		BIT(4)
> +#define MAX9485_CLKOUT1_ENABLE	BIT(5)
> +#define MAX9485_CLKOUT2_ENABLE	BIT(6)
> +#define MAX9485_MCLK_ENABLE	BIT(7)
> +#define MAX9485_FREQ_MASK	0x1f
> +
> +struct max9485_rate {
> +	unsigned long out;
> +	u8 reg_value;
> +};
> +
> +/*
> + * Ordered by frequency. For frequency the hardware can generate with
> + * multiple settings, only the one with lowest jitter is listed.
> + */
> +static const struct max9485_rate max9485_rates[] = {
> +	{  3072000, MAX9485_FS_12KHZ   | MAX9485_SCALE_256 },
> +	{  4608000, MAX9485_FS_12KHZ   | MAX9485_SCALE_384 },
> +	{  8192000, MAX9485_FS_32KHZ   | MAX9485_SCALE_256 },
> +	{  9126000, MAX9485_FS_12KHZ   | MAX9485_SCALE_768 },
> +	{ 11289600, MAX9485_FS_44_1KHZ | MAX9485_SCALE_256 },
> +	{ 12288000, MAX9485_FS_48KHZ   | MAX9485_SCALE_256 },
> +	{ 16384000, MAX9485_FS_32KHZ   | MAX9485_SCALE_256 | MAX9485_DOUBLE },
> +	{ 16934400, MAX9485_FS_44_1KHZ | MAX9485_SCALE_384 },
> +	{ 18384000, MAX9485_FS_48KHZ   | MAX9485_SCALE_384 },
> +	{ 22579200, MAX9485_FS_44_1KHZ | MAX9485_SCALE_256 | MAX9485_DOUBLE },
> +	{ 24576000, MAX9485_FS_48KHZ   | MAX9485_SCALE_256 | MAX9485_DOUBLE },
> +	{ 33868800, MAX9485_FS_44_1KHZ | MAX9485_SCALE_384 | MAX9485_DOUBLE },
> +	{ 36864000, MAX9485_FS_48KHZ   | MAX9485_SCALE_384 | MAX9485_DOUBLE },
> +	{ 49152000, MAX9485_FS_32KHZ   | MAX9485_SCALE_768 | MAX9485_DOUBLE },
> +	{ 67737600, MAX9485_FS_44_1KHZ | MAX9485_SCALE_768 | MAX9485_DOUBLE },
> +	{ 73728000, MAX9485_FS_48KHZ   | MAX9485_SCALE_768 | MAX9485_DOUBLE },
> +	{ } /* sentinel */
> +};
> +
> +struct max9485_driver_data;
> +
> +struct max9485_clk_hw {
> +	struct clk_hw hw;
> +	struct clk_init_data init;
> +	u8 enable_bit;
> +	struct max9485_driver_data *drvdata;
> +};
> +
> +struct max9485_driver_data {
> +	struct clk *xclk;
> +	struct i2c_client *client;
> +	u8 reg_value;
> +	unsigned long clkout_rate;
> +	struct regulator *supply;
> +	struct gpio_desc *reset_gpio;
> +	struct max9485_clk_hw hw[MAX9485_NUM_CLKS];
> +
> +	struct {
> +		struct clk_hw_onecell_data data;
> +		struct clk_hw *hw[MAX9485_NUM_CLKS];
> +	} onecell;
> +};
> +
> +static int max9485_update_bits(struct max9485_driver_data *drvdata,
> +			       u8 mask, u8 value)
> +{
> +	int ret;
> +
> +	drvdata->reg_value &= ~mask;
> +	drvdata->reg_value |= value;
> +
> +	dev_dbg(&drvdata->client->dev,
> +		"updating mask %02x value %02x -> %02x\n",
> +		mask, value, drvdata->reg_value);
> +
> +	ret = i2c_master_send(drvdata->client,
> +			      &drvdata->reg_value,
> +			      sizeof(drvdata->reg_value));
> +
> +	return (ret < 0) ? ret : 0;
> +}
> +
> +static int max9485_clk_prepare(struct clk_hw *hw)
> +{
> +	struct max9485_clk_hw *clk_hw =
> +		container_of(hw, struct max9485_clk_hw, hw);
> +
> +	return max9485_update_bits(clk_hw->drvdata,
> +				   clk_hw->enable_bit,
> +				   clk_hw->enable_bit);
> +}
> +
> +static void max9485_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct max9485_clk_hw *clk_hw =
> +		container_of(hw, struct max9485_clk_hw, hw);
> +
> +	max9485_update_bits(clk_hw->drvdata, clk_hw->enable_bit, 0);
> +}
> +
> +/*
> + * MCLK OUT
> + */
> +
> +/* The MCLK output can only provide the same rate as the input clock */
> +static unsigned long max9485_mclkout_recalc_rate(struct clk_hw *hw,
> +						 unsigned long parent_rate)
> +{
> +	return MAX9485_INPUT_FREQ;
> +}
> +
> +/*
> + * CLKOUT - configurable clock output
> + */
> +static int max9485_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
> +				   unsigned long parent_rate)
> +{
> +	const struct max9485_rate *entry;
> +	struct max9485_clk_hw *clk_hw =
> +		container_of(hw, struct max9485_clk_hw, hw);
> +
> +	for (entry = max9485_rates; entry->out != 0; entry++)
> +		if (entry->out == rate)
> +			break;
> +
> +	if (entry->out == 0)
> +		return -EINVAL;
> +
> +	clk_hw->drvdata->clkout_rate = rate;
> +
> +	return max9485_update_bits(clk_hw->drvdata,
> +				   MAX9485_FREQ_MASK,
> +				   entry->reg_value);
> +}
> +
> +static long max9485_clkout_round_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long *parent_rate)
> +{
> +	const struct max9485_rate *curr, *prev = NULL;
> +
> +	for (curr = max9485_rates; curr->out != 0; curr++) {
> +		/* Exact matches */
> +		if (curr->out == rate)
> +			return rate;
> +
> +		/*
> +		 * Find the first entry that has a frequency higher than the
> +		 * requested one.
> +		 */
> +		if (curr->out > rate) {
> +			unsigned int mid;
> +
> +			/*
> +			 * If this is the first entry, clamp the value to the
> +			 * lowest possible frequency.
> +			 */
> +			if (!prev)
> +				return curr->out;
> +
> +			/*
> +			 * Otherwise, determine whether the previous entry or
> +			 * current one is closer.
> +			 */
> +			mid = prev->out + ((curr->out - prev->out) / 2);
> +
> +			return (mid > rate) ? prev->out : curr->out;
> +		}
> +
> +		prev = curr;
> +	}
> +
> +	/* If the last entry was still too high, clamp the value */
> +	return prev->out;
> +}
> +
> +static unsigned long max9485_clkout_recalc_rate(struct clk_hw *hw,
> +						unsigned long parent_rate)
> +{
> +	struct max9485_clk_hw *clk_hw =
> +		container_of(hw, struct max9485_clk_hw, hw);
> +
> +	return clk_hw->drvdata->clkout_rate;
> +}
> +
> +struct max9485_clk {
> +	const char *name;
> +	int parent_index;
> +	const struct clk_ops ops;
> +	u8 enable_bit;
> +};
> +
> +static const struct max9485_clk max9485_clks[MAX9485_NUM_CLKS] = {
> +	[MAX9485_MCLKOUT] = {
> +		.name = "mclkout",
> +		.parent_index = -1,
> +		.enable_bit = MAX9485_MCLK_ENABLE,
> +		.ops = {
> +			.prepare	= max9485_clk_prepare,
> +			.unprepare	= max9485_clk_unprepare,
> +			.recalc_rate	= max9485_mclkout_recalc_rate,
> +		},
> +	},
> +	[MAX9485_CLKOUT] = {
> +		.name = "clkout",
> +		.parent_index = -1,
> +		.ops = {
> +			.set_rate	= max9485_clkout_set_rate,
> +			.round_rate	= max9485_clkout_round_rate,
> +			.recalc_rate	= max9485_clkout_recalc_rate,
> +		},
> +	},
> +	[MAX9485_CLKOUT1] = {
> +		.name = "clkout1",
> +		.parent_index = MAX9485_CLKOUT,
> +		.enable_bit = MAX9485_CLKOUT1_ENABLE,
> +		.ops = {
> +			.prepare	= max9485_clk_prepare,
> +			.unprepare	= max9485_clk_unprepare,
> +			.recalc_rate	= max9485_clkout_recalc_rate,
> +		},
> +	},
> +	[MAX9485_CLKOUT2] = {
> +		.name = "clkout2",
> +		.parent_index = MAX9485_CLKOUT,
> +		.enable_bit = MAX9485_CLKOUT2_ENABLE,
> +		.ops = {
> +			.prepare	= max9485_clk_prepare,
> +			.unprepare	= max9485_clk_unprepare,
> +			.recalc_rate	= max9485_clkout_recalc_rate,
> +		},
> +	},
> +};
> +
> +static int max9485_i2c_probe(struct i2c_client *client,
> +			     const struct i2c_device_id *id)
> +{
> +	struct max9485_driver_data *drvdata;
> +	struct device *dev = &client->dev;
> +	unsigned long freq;
> +	int i, ret;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (drvdata == NULL)
> +		return -ENOMEM;
> +
> +	drvdata->xclk = devm_clk_get(dev, "xclk");
> +	if (IS_ERR(drvdata->xclk))
> +		return PTR_ERR(drvdata->xclk);
> +
> +	freq = clk_get_rate(drvdata->xclk);
> +	if (freq != MAX9485_INPUT_FREQ) {
> +		dev_err(dev, "Illegal xclk frequency of %ld\n", freq);
> +		return -EINVAL;
> +	}
> +
> +	drvdata->supply = devm_regulator_get(dev, "vdd");
> +	if (IS_ERR(drvdata->supply))
> +		return PTR_ERR(drvdata->supply);
> +
> +	ret = regulator_enable(drvdata->supply);
> +	if (ret < 0)
> +		return ret;
> +
> +	drvdata->reset_gpio =
> +		devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> +	if (IS_ERR(drvdata->reset_gpio))
> +		return PTR_ERR(drvdata->reset_gpio);
> +
> +	i2c_set_clientdata(client, drvdata);
> +	drvdata->client = client;
> +
> +	for (i = 0; i < MAX9485_NUM_CLKS; i++) {
> +		int parent_index = max9485_clks[i].parent_index;
> +		const char *name;
> +
> +		if (of_property_read_string_index(dev->of_node,
> +						  "clock-output-names",
> +						  i, &name) == 0)
> +			drvdata->hw[i].init.name = name;
> +		else
> +			drvdata->hw[i].init.name = max9485_clks[i].name;
> +
> +		drvdata->hw[i].init.ops = &max9485_clks[i].ops;
> +		drvdata->hw[i].init.flags = CLK_IS_BASIC;
> +
> +		if (parent_index > 0) {
> +			drvdata->hw[i].init.parent_names =
> +				&drvdata->hw[parent_index].init.name;
> +			drvdata->hw[i].init.num_parents = 1;
> +			drvdata->hw[i].init.flags |= CLK_SET_RATE_PARENT;
> +		}
> +
> +		drvdata->hw[i].enable_bit = max9485_clks[i].enable_bit;
> +		drvdata->hw[i].hw.init = &drvdata->hw[i].init;
> +		drvdata->hw[i].drvdata = drvdata;
> +
> +		ret = devm_clk_hw_register(dev, &drvdata->hw[i].hw);
> +		if (ret < 0)
> +			return ret;
> +
> +		drvdata->onecell.hw[i] = &drvdata->hw[i].hw;
> +	}
> +
> +	drvdata->onecell.data.num = MAX9485_NUM_CLKS;
> +
> +	return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get,
> +					   &drvdata->onecell.data);
> +}
> +
> +static int __maybe_unused max9485_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct max9485_driver_data *drvdata = i2c_get_clientdata(client);
> +
> +	if (drvdata->reset_gpio)
> +		gpiod_set_value_cansleep(drvdata->reset_gpio, 0);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused max9485_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct max9485_driver_data *drvdata = i2c_get_clientdata(client);
> +	int ret;
> +
> +	if (drvdata->reset_gpio)
> +		gpiod_set_value_cansleep(drvdata->reset_gpio, 0);
> +
> +	ret = i2c_master_send(client, &drvdata->reg_value,
> +			      sizeof(drvdata->reg_value));
> +
> +	return (ret < 0) ? ret : 0;
> +}
> +
> +static const struct dev_pm_ops max9485_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(max9485_suspend, max9485_resume)
> +};
> +
> +static const struct of_device_id max9485_dt_ids[] = {
> +	{ .compatible = "maxim,max9485", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, max9485_dt_ids);
> +
> +static const struct i2c_device_id max9485_i2c_ids[] = {
> +	{ .name = "max9485", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max9485_i2c_ids);
> +
> +static struct i2c_driver max9485_driver = {
> +	.driver = {
> +		.name		= "max9485",
> +		.pm		= &max9485_pm_ops,
> +		.of_match_table	= max9485_dt_ids,
> +	},
> +	.probe = max9485_i2c_probe,
> +	.id_table = max9485_i2c_ids,
> +};
> +module_i2c_driver(max9485_driver);
> +
> +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
> +MODULE_DESCRIPTION("MAX9485 Programmable Audio Clock Generator");
> +MODULE_LICENSE("GPL v2");
> 

^ permalink raw reply

* Re: [PATCH 07/15] arm: dts: exynos: Add missing cooling device properties for CPUs
From: Viresh Kumar @ 2018-05-31  5:22 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: arm, Rob Herring, Mark Rutland, Kukjin Kim, Vincent Guittot,
	ionela.voinescu, Daniel Lezcano, chris.redpath, devicetree,
	linux-arm-kernel, linux-samsung-soc@vger.kernel.org, linux-kernel
In-Reply-To: <CAJKOXPftrzwLMSDeTBBxG-nvpsriBuPEwQAViTcE3DYRYsnzmg@mail.gmail.com>

On 30-05-18, 14:32, Krzysztof Kozlowski wrote:
> OK, I see the possibility although it is still far away from use
> cases. You cannot hotplug booting CPU (CPU0) on Exynos kernels. It
> never worked. Strictly speaking - offlining will work. But bringing it
> online will likely hang the system.

True and I used the following out of tree patch for a long time for my
dual A15 exynos to make hotplug work.

https://git.linaro.org/people/vireshk/backup/linux.git/commit/?h=bkp/exynos/hotplug&id=aab8a906a70b8f1fb15a4b7bd2ee27e6dcabf79d

-- 
viresh

^ permalink raw reply

* Re: [PATCH] ARM: dts: exynos: Add missing CPU clocks to secondary CPUs on Exynos542x
From: Viresh Kumar @ 2018-05-31  5:21 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Mark Rutland, Kukjin Kim, Marek Szyprowski,
	devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel
In-Reply-To: <20180530164922.31851-1-krzk@kernel.org>

On 30-05-18, 18:49, Krzysztof Kozlowski wrote:
> Secondary CPUs should have the same information in DeviceTree as booting
> CPU from both correctness point of view and for possible hotplug
> scenarios.
> 
> Suggested-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm/boot/dts/exynos5420-cpus.dtsi | 6 ++++++
>  arch/arm/boot/dts/exynos5422-cpus.dtsi | 8 +++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)

Nice.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

^ permalink raw reply

* [PATCH v3 5/5] arm64: dts: rockchip: Add sdmmc UHS support for roc-rk3328-cc
From: djw @ 2018-05-31  5:03 UTC (permalink / raw)
  To: linux-rockchip
  Cc: Wayne Chou, Levin Du, Heiko Stuebner, devicetree, David Wu,
	Liang Chen, William Wu, linux-kernel, Rob Herring, Joseph Chen,
	Will Deacon, Rocky Hao, Mark Rutland, Catalin Marinas,
	linux-arm-kernel
In-Reply-To: <1527737273-8387-1-git-send-email-djw@t-chip.com.cn>

From: Levin Du <djw@t-chip.com.cn>

In roc-rk3328-cc board, the signal voltage of sdmmc is supplied by
the vcc_sdio regulator, which is a mux between 1.8V and 3.3V,
controlled by a special output only gpio pin labeled
"gpiomut_pmuio_iout", corresponding bit 1 of the syscon GRF_SOC_CON10.

This special pin can now be reference as <&gpio_mute 0>, thanks
to the gpio-syscon driver, which makes writing regulator-gpio possible.

If the signal voltage changes, the io domain needs to change
correspondingly.

To use this feature, the following options are required in kernel config:
 - CONFIG_GPIO_SYSCON=y
 - CONFIG_POWER_AVS=y
 - CONFIG_ROCKCHIP_IODOMAIN=y

Signed-off-by: Levin Du <djw@t-chip.com.cn>

---

Changes in v3:
- Use <&gpio_mute 0> instead of <&gpio_mute 1> to refer to the GPIO_MUTE pin.

Changes in v2:
- Rename gpio_syscon10 to gpio_mute in rk3328-roc-cc.dts

Changes in v1:
- Split into small patches
- Sort dts properties in sdmmc node

 arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
index b983abd..25c1111 100644
--- a/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3328-roc-cc.dts
@@ -41,6 +41,19 @@
 		vin-supply = <&vcc_io>;
 	};
 
+	vcc_sdio: sdmmcio-regulator {
+		compatible = "regulator-gpio";
+		gpios = <&gpio_mute 0 GPIO_ACTIVE_HIGH>;
+		states = <1800000 0x1
+			  3300000 0x0>;
+		regulator-name = "vcc_sdio";
+		regulator-type = "voltage";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-always-on;
+		vin-supply = <&vcc_sys>;
+	};
+
 	vcc_host1_5v: vcc_otg_5v: vcc-host1-5v-regulator {
 		compatible = "regulator-fixed";
 		enable-active-high;
@@ -213,7 +226,7 @@
 
 	vccio1-supply = <&vcc_io>;
 	vccio2-supply = <&vcc18_emmc>;
-	vccio3-supply = <&vcc_io>;
+	vccio3-supply = <&vcc_sdio>;
 	vccio4-supply = <&vcc_18>;
 	vccio5-supply = <&vcc_io>;
 	vccio6-supply = <&vcc_io>;
@@ -242,7 +255,12 @@
 	max-frequency = <150000000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&sdmmc0_clk &sdmmc0_cmd &sdmmc0_dectn &sdmmc0_bus4>;
+	sd-uhs-sdr12;
+	sd-uhs-sdr25;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	vmmc-supply = <&vcc_sd>;
+	vqmmc-supply = <&vcc_sdio>;
 	status = "okay";
 };
 
@@ -277,3 +295,7 @@
 &usb_host0_ohci {
 	status = "okay";
 };
+
+&gpio_mute {
+	status = "okay";
+};
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH v9 01/15] ARM: Add Krait L2 register accessor functions
From: Sricharan R @ 2018-05-31  4:57 UTC (permalink / raw)
  To: Stephen Boyd, Bjorn Andersson
  Cc: robh, viresh.kumar, mark.rutland, mturquette, sboyd, linux,
	andy.gross, david.brown, rjw, linux-arm-kernel, devicetree,
	linux-kernel, linux-clk, linux-arm-msm, linux-soc, linux-pm,
	linux
In-Reply-To: <152769571081.144038.4314499217001219157@swboyd.mtv.corp.google.com>

Hi Stephen,

On 5/30/2018 9:25 PM, Stephen Boyd wrote:
> Quoting Sricharan R (2018-05-24 22:40:11)
>> Hi Bjorn,
>>
>> On 5/24/2018 11:09 PM, Bjorn Andersson wrote:
>>> On Tue 06 Mar 06:38 PST 2018, Sricharan R wrote:
>>>
>>>> From: Stephen Boyd <sboyd@codeaurora.org>
>>>>
>>>> Krait CPUs have a handful of L2 cache controller registers that
>>>> live behind a cp15 based indirection register. First you program
>>>> the indirection register (l2cpselr) to point the L2 'window'
>>>> register (l2cpdr) at what you want to read/write.  Then you
>>>> read/write the 'window' register to do what you want. The
>>>> l2cpselr register is not banked per-cpu so we must lock around
>>>> accesses to it to prevent other CPUs from re-pointing l2cpdr
>>>> underneath us.
>>>>
>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>> Cc: Russell King <linux@arm.linux.org.uk>
>>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>>
>>> This should have your signed-off-by here as well.
>>>
>>
>>  ok.
>>
>>> Apart from that:
>>>
>>> Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>>
>>
> 
> Will these patches come around again? I'll do a quick sweep on them
> today but I expect them to be resent.

Sure, i will have to resend them again, fixing couple of Bjorn's
minor comments. Will address your comments that you would give
as well along with that.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

^ permalink raw reply

* Re: [PATCH v2 1/6] soc: qcom: rpmpd: Add a powerdomain driver to model corners
From: Rajendra Nayak @ 2018-05-31  4:20 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Viresh Kumar, Stephen Boyd, Andy Gross, devicetree, linux-arm-msm,
	Linux Kernel Mailing List, collinsd
In-Reply-To: <CAPDyKFpuZ4s7C+Nr3NhU-A40oVw84yK7eEniO6NhHNJ3VosR-A@mail.gmail.com>



On 05/30/2018 06:14 PM, Ulf Hansson wrote:
> [...]
> 
>>>> +
>>>> +static DEFINE_MUTEX(rpmpd_lock);
>>>> +
>>>> +/* msm8996 RPM powerdomains */
>>>> +DEFINE_RPMPD_CORN_SMPA(msm8996, vddcx, vddcx_ao, 1);
>>>> +DEFINE_RPMPD_CORN_SMPA(msm8996, vddmx, vddmx_ao, 2);
>>>> +DEFINE_RPMPD_CORN_LDOA(msm8996, vddsscx, 26);
>>>> +
>>>> +DEFINE_RPMPD_VFC_SMPA(msm8996, vddcx_vfc, 1);
>>>> +DEFINE_RPMPD_VFC_LDOA(msm8996, vddsscx_vfc, 26);
>>>> +
>>>> +static struct rpmpd *msm8996_rpmpds[] = {
>>>> +       [0] = &msm8996_vddcx,
>>>> +       [1] = &msm8996_vddcx_ao,
>>>> +       [2] = &msm8996_vddcx_vfc,
>>>> +       [3] = &msm8996_vddmx,
>>>> +       [4] = &msm8996_vddmx_ao,
>>>> +       [5] = &msm8996_vddsscx,
>>>> +       [6] = &msm8996_vddsscx_vfc,
>>>> +};
>>>
>>> It's not my call, but honestly the above all macros makes the code
>>> less readable.
>>
>> This is all static data per SoC. The macros will keep the new additions
>> needed for every new SoC to a minimal. Currently this supports only
>> msm8996.
> 
> Right, that's fine then.
> 
>>
>>>
>>> Anyway, I think you should convert to allocate these structs
>>> dynamically from the heap (kzalloc/kcalloc), instead of statically as
>>> above.
> 
> However, I assume this is still doable!?

Perhaps it is, but is there any specific advantage of constructing these structures
dynamically vs statically, given they are static data?
Most other powerdomain/clock/regulator drivers I see do it statically, and thats
what I followed.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply


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