Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
From: Heiko Stübner @ 2017-01-23 17:12 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Alexander Kochetkov, LKML, devicetree, LAK, linux-rockchip,
	Thomas Gleixner, Mark Rutland, Rob Herring, Russell King,
	Caesar Wang, Huang Tao
In-Reply-To: <20170123144744.GE2166@mai>

Am Montag, 23. Januar 2017, 15:47:44 CET schrieb Daniel Lezcano:
> On Wed, Dec 21, 2016 at 05:21:05PM +0300, Alexander Kochetkov wrote:
> > Hello Heiko, Daniel!
> > 
> > Are there any reasons why the patches [1][2] are not applied yet into
> > kernel? How can I help in applying patches?
> 
> Hi Alexander,
> 
> sorry for the delay. Let me review them before taking the patchset.
> 
> Heiko, can you have a look to them also ?

somehow this series moved down to much in my inbox, sorry.

Devicetree changes look good, except where I commented.
I guess it would be best (least intrusive) if I queue up the (then fixed) 
devicetree changes after you are satisfied with the code parts.

I've also looked over the code changes again, test-build them and found the 
build error mentioned separately. Overall they look good though.


Heiko

^ permalink raw reply

* Re: [PATCH v2 8/9] ARM: dts: add support for I2SE Duckbill 2 SPI
From: Stefan Wahren @ 2017-01-23 17:16 UTC (permalink / raw)
  To: Michael Heimpold, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
In-Reply-To: <1485123764-14754-9-git-send-email-mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>

Am 22.01.2017 um 23:22 schrieb Michael Heimpold:
> This machine is based on I2SE's Duckbill 2 board and is sold as part of I2SE's
> PLC Bundle for IoT. This is a development kit for Homeplug Green PHY based
> powerline products based on Qualcomms QCA7000 chip.
>
> Signed-off-by: Michael Heimpold <mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>
> Cc: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile                 |   1 +
>  arch/arm/boot/dts/imx28-duckbill-2-spi.dts | 199 +++++++++++++++++++++++++++++
>  2 files changed, 200 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx28-duckbill-2-spi.dts
>
...
> diff --git a/arch/arm/boot/dts/imx28-duckbill-2-spi.dts b/arch/arm/boot/dts/imx28-duckbill-2-spi.dts
> new file mode 100644
> index 0000000..738eaa7
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx28-duckbill-2-spi.dts
> @@ -0,0 +1,199 @@
> +/*
> + * Copyright (C) 2015-2017 I2SE GmbH <info-eS4NqCHxEME@public.gmane.org>
> + * Copyright (C) 2016 Michael Heimpold <mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include "imx28.dtsi"
> +
> +/ {
> +	model = "I2SE Duckbill 2 SPI";
> +	compatible = "i2se,duckbill", "fsl,imx28";
> +
> +	aliases {
> +		ethernet1 = &qca7000;
> +	};
> +
> +	memory {
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	apb@80000000 {
> +		apbh@80000000 {
> +			ssp0: ssp@80010000 {
> +				compatible = "fsl,imx28-mmc";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&mmc0_8bit_pins_a
> +					&mmc0_cd_cfg &mmc0_sck_cfg>;
> +				bus-width = <8>;
> +				vmmc-supply = <&reg_3p3v>;
> +				status = "okay";
> +				non-removable;
> +			};
> +
> +			ssp2: ssp@80014000 {
> +				compatible = "fsl,imx28-spi";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&spi2_pins_a>;
> +				status = "okay";
> +
> +				qca7000: ethernet@0 {
> +					compatible = "qca,qca7000";
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&qca7000_pins>;
> +					reg = <0>;

Just a nit. Please place the reg property before the compatible in order
to keep it like the others.

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

^ permalink raw reply

* Re: [PATCH v2 9/9] Documentation: DT: add entries for I2SE boards
From: Stefan Wahren @ 2017-01-23 17:20 UTC (permalink / raw)
  To: Michael Heimpold, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Fabio Estevam
In-Reply-To: <1485123764-14754-10-git-send-email-mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>

Am 22.01.2017 um 23:22 schrieb Michael Heimpold:
> This patch adds the missing devicetree binding documentation for I2SE's
> Duckbill and Duckbill 2 series boards.
>
> Signed-off-by: Michael Heimpold <mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>
> Cc: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/i2se.txt | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/i2se.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/i2se.txt b/Documentation/devicetree/bindings/arm/i2se.txt
> new file mode 100644
> index 0000000..9f30382
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/i2se.txt
> @@ -0,0 +1,22 @@
> +I2SE Device Tree Bindings
> +-------------------------
> +
> +Duckbill Board
> +Required root node properties:
> +    - compatible = "i2se,duckbill", "fsl.imx28";
> +
> +Duckbill 2 Board
> +Required root node properties:
> +    - compatible = "i2se,duckbill", "fsl.imx28";
> +
> +Duckbill 2 485 Board
> +Required root node properties:
> +    - compatible = "i2se,duckbill", "fsl.imx28";
> +
> +Duckbill 2 EnOcean Board
> +Required root node properties:
> +    - compatible = "i2se,duckbill", "fsl.imx28";
> +
> +Duckbill 2 SPI Board
> +Required root node properties:
> +    - compatible = "i2se,duckbill", "fsl.imx28";

Please replace all fsl.imx28 with fsl,imx28

Maybe this patch should come before adding the Duckbill 2 board dts files.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
From: Alexander Kochetkov @ 2017-01-23 17:24 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: Daniel Lezcano, LKML, devicetree-u79uwXL29TY76Z2rM5mHXA, LAK,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Thomas Gleixner,
	Mark Rutland, Rob Herring, Russell King, Caesar Wang, Huang Tao
In-Reply-To: <1793912.WfGyM7qEjy@diego>

Heiko, Daniel, thanks a lot for review!
I’ll send v5 series this week.

Heiko, Daniel, may I add 'Reviewed-by:’ to patch series?

Reviewed-by: Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> ?
Reviewed-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> ?

Regards,
Alexander.

> 23 янв. 2017 г., в 20:12, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> написал(а):
> 
> Am Montag, 23. Januar 2017, 15:47:44 CET schrieb Daniel Lezcano:
>> On Wed, Dec 21, 2016 at 05:21:05PM +0300, Alexander Kochetkov wrote:
>>> Hello Heiko, Daniel!
>>> 
>>> Are there any reasons why the patches [1][2] are not applied yet into
>>> kernel? How can I help in applying patches?
>> 
>> Hi Alexander,
>> 
>> sorry for the delay. Let me review them before taking the patchset.
>> 
>> Heiko, can you have a look to them also ?
> 
> somehow this series moved down to much in my inbox, sorry.
> 
> Devicetree changes look good, except where I commented.
> I guess it would be best (least intrusive) if I queue up the (then fixed) 
> devicetree changes after you are satisfied with the code parts.
> 
> I've also looked over the code changes again, test-build them and found the 
> build error mentioned separately. Overall they look good though.
> 
> 
> Heiko

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

^ permalink raw reply

* Re: [PATCH v3 2/5] drm/rockchip/dsi: dw-mipi: support RK3399 mipi dsi
From: Sean Paul @ 2017-01-23 17:30 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders-F7+t8E8rja9g9hUCZPvPmw, tfiga-F7+t8E8rja9g9hUCZPvPmw,
	heiko-4mtYJXux2i+zQB+pC5nmwQ, yzq-TNX95d0MmH7DzftRWevZcw,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, galak-sgV2jX0FEOL9JmXXK+q4OQ,
	pawel.moll-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1484907051-7159-3-git-send-email-zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Fri, Jan 20, 2017 at 06:10:48PM +0800, Chris Zhong wrote:
> The vopb/vopl switch register of RK3399 mipi is different from RK3288,
> the default setting for mipi dsi mode is different too, so add a
> of_device_id structure to distinguish them, and make sure set the
> correct mode before mipi phy init.
> 

Hi Chris,
There are a bunch of unrelated changes in this patch. Can you please split out
the whitespace/const changes into a separate clean-up patch so it's easier to
see the meaningful changes?

More comments below.

> Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Mark Yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
> Changes in v3:
> - base on John Keeping's patch series
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 101 ++++++++++++++++++++++++---------
>  1 file changed, 74 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 45af890..a93ce97 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -29,9 +29,17 @@
>  
>  #define DRIVER_NAME    "dw-mipi-dsi"
>  
> -#define GRF_SOC_CON6                    0x025c
> -#define DSI0_SEL_VOP_LIT                (1 << 6)
> -#define DSI1_SEL_VOP_LIT                (1 << 9)
> +#define RK3288_GRF_SOC_CON6		0x025c
> +#define RK3288_DSI0_SEL_VOP_LIT		BIT(6)
> +#define RK3288_DSI1_SEL_VOP_LIT		BIT(9)
> +
> +#define RK3399_GRF_SOC_CON19		0x6250
> +#define RK3399_DSI0_SEL_VOP_LIT		BIT(0)
> +#define RK3399_DSI1_SEL_VOP_LIT		BIT(4)
> +
> +/* disable turnrequest, turndisable, forcetxstopmode, forcerxmode */
> +#define RK3399_GRF_SOC_CON22		0x6258
> +#define RK3399_GRF_DSI_MODE		0xffff0000
>  
>  #define DSI_VERSION			0x00
>  #define DSI_PWR_UP			0x04
> @@ -149,7 +157,6 @@
>  #define LPRX_TO_CNT(p)			((p) & 0xffff)
>  
>  #define DSI_BTA_TO_CNT			0x8c
> -

Unrelated whitespace change

>  #define DSI_LPCLK_CTRL			0x94
>  #define AUTO_CLKLANE_CTRL		BIT(1)
>  #define PHY_TXREQUESTCLKHS		BIT(0)
> @@ -215,11 +222,11 @@
>  
>  #define HSFREQRANGE_SEL(val)	(((val) & 0x3f) << 1)
>  
> -#define INPUT_DIVIDER(val)	((val - 1) & 0x7f)
> +#define INPUT_DIVIDER(val)	(((val) - 1) & 0x7f)

Unrelated change

>  #define LOW_PROGRAM_EN		0
>  #define HIGH_PROGRAM_EN		BIT(7)
> -#define LOOP_DIV_LOW_SEL(val)	((val - 1) & 0x1f)
> -#define LOOP_DIV_HIGH_SEL(val)	(((val - 1) >> 5) & 0x1f)
> +#define LOOP_DIV_LOW_SEL(val)	(((val) - 1) & 0x1f)
> +#define LOOP_DIV_HIGH_SEL(val)	((((val) - 1) >> 5) & 0x1f)

This change doesn't adversely affect other platforms?

>  #define PLL_LOOP_DIV_EN		BIT(5)
>  #define PLL_INPUT_DIV_EN	BIT(4)
>  
> @@ -265,6 +272,11 @@ enum {
>  };
>  
>  struct dw_mipi_dsi_plat_data {
> +	u32 dsi0_en_bit;
> +	u32 dsi1_en_bit;
> +	u32 grf_switch_reg;
> +	u32 grf_dsi0_mode;
> +	u32 grf_dsi0_mode_reg;
>  	unsigned int max_data_lanes;
>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>  					   struct drm_display_mode *mode);
> @@ -281,6 +293,7 @@ struct dw_mipi_dsi {
>  
>  	struct clk *pllref_clk;
>  	struct clk *pclk;
> +	struct clk *phy_cfg_clk;
>  
>  	unsigned int lane_mbps; /* per lane */
>  	u32 channel;
> @@ -356,6 +369,7 @@ static inline struct dw_mipi_dsi *encoder_to_dsi(struct drm_encoder *encoder)
>  {
>  	return container_of(encoder, struct dw_mipi_dsi, encoder);
>  }
> +
>  static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
>  {
>  	writel(val, dsi->base + reg);
> @@ -367,7 +381,7 @@ static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
>  }
>  
>  static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi *dsi, u8 test_code,
> -				 u8 test_data)
> +				  u8 test_data)

Unrelated whitespace change

>  {
>  	/*
>  	 * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content
> @@ -426,6 +440,14 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
>  	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
>  
> +	if (!IS_ERR(dsi->phy_cfg_clk)) {
> +		ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +		if (ret) {
> +			dev_err(dsi->dev, "Failed to enable phy_cfg_clk\n");
> +			return ret;
> +		}
> +	}
> +
>  	dw_mipi_dsi_phy_write(dsi, 0x10, BYPASS_VCO_RANGE |
>  					 VCO_RANGE_CON_SEL(vco) |
>  					 VCO_IN_CAP_CON_LOW |
> @@ -474,22 +496,23 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
>  	dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
>  				     PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
>  
> -
>  	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
>  				 val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US);
>  	if (ret < 0) {
>  		dev_err(dsi->dev, "failed to wait for phy lock state\n");
> -		return ret;
> +		goto phy_init_end;
>  	}
>  
>  	ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
>  				 val, val & STOP_STATE_CLK_LANE, 1000,
>  				 PHY_STATUS_TIMEOUT_US);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dsi->dev,
>  			"failed to wait for phy clk lane stop state\n");
> -		return ret;
> -	}
> +
> +phy_init_end:
> +	if (!IS_ERR(dsi->phy_cfg_clk))
> +		clk_disable_unprepare(dsi->phy_cfg_clk);
>  
>  	return ret;
>  }
> @@ -548,7 +571,7 @@ static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
>  
>  	if (device->lanes > dsi->pdata->max_data_lanes) {
>  		dev_err(dsi->dev, "the number of data lanes(%u) is too many\n",
> -				device->lanes);
> +			device->lanes);

Unrelated whitespace change

>  		return -EINVAL;
>  	}
>  
> @@ -936,8 +959,8 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
>  }
>  
>  static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder,
> -					struct drm_display_mode *mode,
> -					struct drm_display_mode *adjusted_mode)
> +					 struct drm_display_mode *mode,
> +					 struct drm_display_mode *adjusted_mode)

Unrelated whitespace change

>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>  
> @@ -965,6 +988,7 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> +	const struct dw_mipi_dsi_plat_data *pdata = dsi->pdata;
>  	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>  	u32 val;
>  
> @@ -985,6 +1009,10 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	dw_mipi_dsi_dphy_interface_config(dsi);
>  	dw_mipi_dsi_clear_err(dsi);
>  
> +	if (pdata->grf_dsi0_mode_reg)
> +		regmap_write(dsi->grf_regmap, pdata->grf_dsi0_mode_reg,
> +			     pdata->grf_dsi0_mode);
> +
>  	dw_mipi_dsi_phy_init(dsi);
>  	dw_mipi_dsi_wait_for_two_frames(dsi);
>  
> @@ -998,11 +1026,11 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	clk_disable_unprepare(dsi->pclk);
>  
>  	if (mux)
> -		val = DSI0_SEL_VOP_LIT | (DSI0_SEL_VOP_LIT << 16);
> +		val = pdata->dsi0_en_bit | (pdata->dsi0_en_bit << 16);
>  	else
> -		val = DSI0_SEL_VOP_LIT << 16;
> +		val = pdata->dsi0_en_bit << 16;
>  
> -	regmap_write(dsi->grf_regmap, GRF_SOC_CON6, val);
> +	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
>  	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
>  }
>  
> @@ -1034,7 +1062,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder,
>  	return 0;
>  }
>  
> -static struct drm_encoder_helper_funcs
> +static const struct drm_encoder_helper_funcs
>  dw_mipi_dsi_encoder_helper_funcs = {
>  	.enable = dw_mipi_dsi_encoder_enable,
>  	.mode_set = dw_mipi_dsi_encoder_mode_set,
> @@ -1042,7 +1070,7 @@ dw_mipi_dsi_encoder_helper_funcs = {
>  	.atomic_check = dw_mipi_dsi_encoder_atomic_check,
>  };
>  
> -static struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
> +static const struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = {
>  	.destroy = drm_encoder_cleanup,
>  };
>  
> @@ -1078,7 +1106,7 @@ static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
>  	drm_connector_cleanup(connector);
>  }
>  
> -static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
> +static const struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {

These 3 const changes are unrelated to the patch

>  	.dpms = drm_atomic_helper_connector_dpms,
>  	.fill_modes = drm_helper_probe_single_connector_modes,
>  	.destroy = dw_mipi_dsi_drm_connector_destroy,
> @@ -1088,7 +1116,7 @@ static struct drm_connector_funcs dw_mipi_dsi_atomic_connector_funcs = {
>  };
>  
>  static int dw_mipi_dsi_register(struct drm_device *drm,
> -				      struct dw_mipi_dsi *dsi)
> +				struct dw_mipi_dsi *dsi)

Unrelated whitespace change

>  {
>  	struct drm_encoder *encoder = &dsi->encoder;
>  	struct drm_connector *connector = &dsi->connector;
> @@ -1109,14 +1137,14 @@ static int dw_mipi_dsi_register(struct drm_device *drm,
>  	drm_encoder_helper_add(&dsi->encoder,
>  			       &dw_mipi_dsi_encoder_helper_funcs);
>  	ret = drm_encoder_init(drm, &dsi->encoder, &dw_mipi_dsi_encoder_funcs,
> -			 DRM_MODE_ENCODER_DSI, NULL);
> +			       DRM_MODE_ENCODER_DSI, NULL);

Unrelated whitespace change

>  	if (ret) {
>  		dev_err(dev, "Failed to initialize encoder with drm\n");
>  		return ret;
>  	}
>  
>  	drm_connector_helper_add(connector,
> -			&dw_mipi_dsi_connector_helper_funcs);
> +				 &dw_mipi_dsi_connector_helper_funcs);

Unrelated whitespace change

>  
>  	drm_connector_init(drm, &dsi->connector,
>  			   &dw_mipi_dsi_atomic_connector_funcs,
> @@ -1162,21 +1190,36 @@ static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
>  }
>  
>  static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
> +	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
> +	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
> +	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>  	.max_data_lanes = 4,
>  	.mode_valid = rk3288_mipi_dsi_mode_valid,
>  };
>  
> +static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> +	.dsi0_en_bit = RK3399_DSI0_SEL_VOP_LIT,
> +	.dsi1_en_bit = RK3399_DSI1_SEL_VOP_LIT,
> +	.grf_switch_reg = RK3399_GRF_SOC_CON19,
> +	.grf_dsi0_mode = RK3399_GRF_DSI_MODE,
> +	.grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22,
> +	.max_data_lanes = 4,
> +};
> +
>  static const struct of_device_id dw_mipi_dsi_dt_ids[] = {
>  	{
>  	 .compatible = "rockchip,rk3288-mipi-dsi",
>  	 .data = &rk3288_mipi_dsi_drv_data,
> +	}, {
> +	 .compatible = "rockchip,rk3399-mipi-dsi",
> +	 .data = &rk3399_mipi_dsi_drv_data,
>  	},
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, dw_mipi_dsi_dt_ids);
>  
>  static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
> -			     void *data)
> +			    void *data)

Unrelated whitespace change

>  {
>  	const struct of_device_id *of_id =
>  			of_match_device(dw_mipi_dsi_dt_ids, dev);
> @@ -1249,6 +1292,10 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  		clk_disable_unprepare(dsi->pclk);
>  	}
>  
> +	dsi->phy_cfg_clk = devm_clk_get(dev, "phy_cfg");
> +	if (IS_ERR(dsi->phy_cfg_clk))
> +		dev_dbg(dev, "have not phy_cfg_clk\n");
> +
>  	ret = clk_prepare_enable(dsi->pllref_clk);
>  	if (ret) {
>  		dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
> @@ -1280,7 +1327,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  }
>  
>  static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
> -	void *data)
> +			       void *data)

Unrelated whitespace change

>  {
>  	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

^ permalink raw reply

* Re: [PATCH v3 3/5] drm/rockchip/dsi: remove mode_valid function
From: Sean Paul @ 2017-01-23 17:48 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders, tfiga, heiko, yzq, mark.rutland, devicetree, robh+dt,
	galak, pawel.moll, linux-kernel, dri-devel, linux-rockchip,
	linux-arm-kernel
In-Reply-To: <1484907051-7159-4-git-send-email-zyw@rock-chips.com>

On Fri, Jan 20, 2017 at 06:10:49PM +0800, Chris Zhong wrote:
> The MIPI DSI do not need check the validity of resolution, the max
> resolution should depend VOP. Hence, remove rk3288_mipi_dsi_mode_valid
> here.

Does vop actually enforce this, though? I see that mode_config.max_width is
4096, but there is no bounds checking in mode_fixup().

The connector is currently rejecting everything greater than 2047. So I think
you're going to regress behavior here.

Sean


> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
> Changes in v3: None
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 39 ----------------------------------
>  1 file changed, 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index a93ce97..6f0e252 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -278,8 +278,6 @@ struct dw_mipi_dsi_plat_data {
>  	u32 grf_dsi0_mode;
>  	u32 grf_dsi0_mode_reg;
>  	unsigned int max_data_lanes;
> -	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
> -					   struct drm_display_mode *mode);
>  };
>  
>  struct dw_mipi_dsi {
> @@ -1081,23 +1079,8 @@ static int dw_mipi_dsi_connector_get_modes(struct drm_connector *connector)
>  	return drm_panel_get_modes(dsi->panel);
>  }
>  
> -static enum drm_mode_status dw_mipi_dsi_mode_valid(
> -					struct drm_connector *connector,
> -					struct drm_display_mode *mode)
> -{
> -	struct dw_mipi_dsi *dsi = con_to_dsi(connector);
> -
> -	enum drm_mode_status mode_status = MODE_OK;
> -
> -	if (dsi->pdata->mode_valid)
> -		mode_status = dsi->pdata->mode_valid(connector, mode);
> -
> -	return mode_status;
> -}
> -
>  static struct drm_connector_helper_funcs dw_mipi_dsi_connector_helper_funcs = {
>  	.get_modes = dw_mipi_dsi_connector_get_modes,
> -	.mode_valid = dw_mipi_dsi_mode_valid,
>  };
>  
>  static void dw_mipi_dsi_drm_connector_destroy(struct drm_connector *connector)
> @@ -1168,33 +1151,11 @@ static int rockchip_mipi_parse_dt(struct dw_mipi_dsi *dsi)
>  	return 0;
>  }
>  
> -static enum drm_mode_status rk3288_mipi_dsi_mode_valid(
> -					struct drm_connector *connector,
> -					struct drm_display_mode *mode)
> -{
> -	/*
> -	 * The VID_PKT_SIZE field in the DSI_VID_PKT_CFG
> -	 * register is 11-bit.
> -	 */
> -	if (mode->hdisplay > 0x7ff)
> -		return MODE_BAD_HVALUE;
> -
> -	/*
> -	 * The V_ACTIVE_LINES field in the DSI_VTIMING_CFG
> -	 * register is 11-bit.
> -	 */
> -	if (mode->vdisplay > 0x7ff)
> -		return MODE_BAD_VVALUE;
> -
> -	return MODE_OK;
> -}
> -
>  static struct dw_mipi_dsi_plat_data rk3288_mipi_dsi_drv_data = {
>  	.dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT,
>  	.dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT,
>  	.grf_switch_reg = RK3288_GRF_SOC_CON6,
>  	.max_data_lanes = 4,
> -	.mode_valid = rk3288_mipi_dsi_mode_valid,
>  };
>  
>  static struct dw_mipi_dsi_plat_data rk3399_mipi_dsi_drv_data = {
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply

* Re: [PATCH v4 1/8] devicetree: property-units: add mWh and mAh units
From: Rob Herring @ 2017-01-23 17:50 UTC (permalink / raw)
  To: Matt Ranostay; +Cc: linux-pm, devicetree, sre, tony
In-Reply-To: <20170122071404.9654-2-matt@ranostay.consulting>

On Sat, Jan 21, 2017 at 11:13:57PM -0800, Matt Ranostay wrote:
> Add entries for microwatt-hours and microamp-hours to property
> units.
> 
> Signed-off-by: Matt Ranostay <matt@ranostay.consulting>

Please add acks when posting new versions. You're missing Sebastian's 
and mine.

> ---
>  Documentation/devicetree/bindings/property-units.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/property-units.txt b/Documentation/devicetree/bindings/property-units.txt
> index 12278d79f6c0..5e8d220cc2b6 100644
> --- a/Documentation/devicetree/bindings/property-units.txt
> +++ b/Documentation/devicetree/bindings/property-units.txt
> @@ -25,8 +25,10 @@ Distance
>  Electricity
>  ----------------------------------------
>  -microamp	: micro amps
> +-microamp-hours : micro amp hours
>  -ohms		: Ohms
>  -micro-ohms	: micro Ohms
> +-microwatt-hours: micro Watt hours
>  -microvolt	: micro volts
>  
>  Temperature
> -- 
> 2.10.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 5/7] ASoC: dmaengine_pcm: add copy support
From: Mark Brown @ 2017-01-23 17:50 UTC (permalink / raw)
  To: Arnaud Pouliquen
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Lee Jones, Rob Herring,
	Mark Rutland, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jaroslav Kysela,
	Takashi Iwai, Liam Girdwood, Maxime Coquelin, Alexandre Torgue
In-Reply-To: <1485189145-29576-6-git-send-email-arnaud.pouliquen-qxv4g6HH51o@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

On Mon, Jan 23, 2017 at 05:32:23PM +0100, Arnaud Pouliquen wrote:
> From: olivier moysan <olivier.moysan-qxv4g6HH51o@public.gmane.org>

> Signed-off-by: olivier moysan <omoysan.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The signoff and author information don't match for this patch, they
should match.  The signoff is important for legal reasons - please see
SubmittingPatches for details.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v3 2/2] clocksource: Add renesas-ostm timer driver
From: Daniel Lezcano @ 2017-01-23 17:52 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Rob Herring, Mark Rutland, Simon Horman, Magnus Damm,
	Russell King, Thomas Gleixner, Geert Uytterhoeven,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170123135423.28780-3-chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>

On Mon, Jan 23, 2017 at 08:54:23AM -0500, Chris Brandt wrote:
> This patch adds a OSTM driver for the Renesas architecture.

As it is a new driver, please give technical details for the log.
 
Replace ioread/write by readl/writel in the code.

> Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
> 
> ---
> v2:
> * changed implementation to be independent channel nodes
> ---
>  arch/arm/mach-shmobile/Kconfig     |   1 +
>  drivers/clocksource/Kconfig        |  12 ++
>  drivers/clocksource/Makefile       |   1 +
>  drivers/clocksource/renesas-ostm.c | 349 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 363 insertions(+)
>  create mode 100644 drivers/clocksource/renesas-ostm.c
> 
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 2bb4b09..b928634 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -57,6 +57,7 @@ config ARCH_R7S72100
>  	select PM
>  	select PM_GENERIC_DOMAINS
>  	select SYS_SUPPORTS_SH_MTU2
> +	select SYS_SUPPORTS_RENESAS_OSTM

- select SYS_SUPPORTS_RENESAS_OSTM
+ select RENESAS_OSTM

>  
>  config ARCH_R8A73A4
>  	bool "R-Mobile APE6 (R8A73A40)"
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 4866f7a..95c8d56 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -431,6 +431,9 @@ config MTK_TIMER
>  config SYS_SUPPORTS_SH_MTU2
>          bool
>  
> +config SYS_SUPPORTS_RENESAS_OSTM
> +        bool
> +

-config SYS_SUPPORTS_RENESAS_OSTM
-        bool
-

>  config SYS_SUPPORTS_SH_TMU
>          bool
>  
> @@ -467,6 +470,15 @@ config SH_TIMER_MTU2
>  	  Timer Pulse Unit 2 (MTU2) hardware available on SoCs from Renesas.
>  	  This hardware comes with 16 bit-timer registers.
>  
> +config RENESAS_OSTM
> +	bool "Renesas OSTM timer driver" if COMPILE_TEST
> +	depends on GENERIC_CLOCKEVENTS
> +	select CLKSRC_MMIO
> +	default SYS_SUPPORTS_RENESAS_OSTM

- default SYS_SUPPORTS_RENESAS_OSTM

Except I missing something, I don' the point of having this intermediate
config option.

> +	help
> +	  This enables the build of the OSTM timer driver.
> +	  It creates a clock source and clock event device.
> +
>  config SH_TIMER_TMU
>  	bool "Renesas TMU timer driver" if COMPILE_TEST
>  	depends on GENERIC_CLOCKEVENTS
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index a14111e..bbd163b 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
>  obj-$(CONFIG_CLKSRC_JCORE_PIT)		+= jcore-pit.o
>  obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
>  obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
> +obj-$(CONFIG_RENESAS_OSTM)	+= renesas-ostm.o
>  obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
>  obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
>  obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
> diff --git a/drivers/clocksource/renesas-ostm.c b/drivers/clocksource/renesas-ostm.c
> new file mode 100644
> index 0000000..37f2461
> --- /dev/null
> +++ b/drivers/clocksource/renesas-ostm.c
> @@ -0,0 +1,349 @@
> +/*
> + * Renesas Timer Support - OSTM
> + *
> + * 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
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>

Please remove everything module related here and below. This driver is not
supposed to be removed and it is compiled in with the Kconfig option.

> +#include <linux/pm_runtime.h>
> +#include <linux/sched_clock.h>
> +
> +/*
> + * The OSTM contains independent channels.
> + * The first OSTM channel probed will be set up as a free running
> + * clocksource. Additionally we will use this clocksource for the system
> + * schedule timer sched_clock().
> + *
> + * The second (or more) channel probed will be set up as an interrupt
> + * driven clock event.
> + */
> +
> +struct ostm_device {
> +	struct platform_device *pdev;
> +	int irq;
> +	struct clk *clk;
> +	unsigned long rate;
> +	void __iomem *base;
> +	unsigned long ticks_per_jiffy;
> +	struct clock_event_device ced;
> +};

You can probably reduce the size of this structure by removing some fields
which are used only at init time.

> +
> +static void __iomem *system_clock;	/* For sched_clock() */
> +
> +/* OSTM REGISTERS */
> +#define	OSTM_CMP		0x000	/* RW,32 */
> +#define	OSTM_CNT		0x004	/* R,32 */
> +#define	OSTM_TE			0x010	/* R,8 */
> +#define	OSTM_TS			0x014	/* W,8 */
> +#define	OSTM_TT			0x018	/* W,8 */
> +#define	OSTM_CTL		0x020	/* RW,8 */
> +
> +#define	TE			0x01
> +#define	TS			0x01
> +#define	TT			0x01
> +#define	CTL_PERIODIC		0x00
> +#define	CTL_ONESHOT		0x02
> +#define	CTL_FREERUN		0x02
> +
> +static struct ostm_device *ced_to_ostm(struct clock_event_device *ced)
> +{
> +	return container_of(ced, struct ostm_device, ced);
> +}
> +
> +static int __init ostm_init_clksrc(struct ostm_device *ostm)
> +{
> +	int ret;
> +
> +	/* irq not used (clock sources don't use interrupts) */
> +
> +	/* stop counter */

One line comment is usually in the network code. Please use multiline.

/*
 * Bla bla
 */

> +	iowrite8(TT, ostm->base + OSTM_TT);
> +	while (ioread8(ostm->base + OSTM_TE) & TE)
> +		;

Comment here for this dangerous loop. Explain why we can't be stuck here.

> +
> +	/* setup as freerun */
> +	iowrite32(0, ostm->base + OSTM_CMP);
> +	iowrite8(CTL_FREERUN, ostm->base + OSTM_CTL);
> +	iowrite8(TS, ostm->base + OSTM_TS);
> +
> +	/* register */
> +	ret = clocksource_mmio_init(ostm->base + OSTM_CNT,
> +			"ostm", ostm->rate,
> +			300, 32, clocksource_mmio_readl_up);
> +
> +	return ret;

return clocksource_mmio_init(...);

> +}
> +
> +static u64 notrace ostm_read_sched_clock(void)
> +{
> +	return ioread32(system_clock);
> +}
> +
> +static int __init ostm_init_sched_clock(struct ostm_device *ostm)
> +{
> +	unsigned long flags;
> +
> +	system_clock = ostm->base + OSTM_CNT;
> +	local_irq_save(flags);
> +	local_irq_disable();

1. local_irq_disable() is not needed, local_irq_save() saves the irq flags and
disables the irq.

2. Why do you need to disable the irq here ?

> +	sched_clock_register(ostm_read_sched_clock, 32, ostm->rate);
> +	local_irq_restore(flags);
> +
> +	return 0;
> +}
> +
> +static void ostm_clkevt_timer_stop(struct ostm_device *ostm)
> +{
> +	if (ioread8(ostm->base + OSTM_TE) & TE) {
> +		iowrite8(TT, ostm->base + OSTM_TT);
> +		while (ioread8(ostm->base + OSTM_TE) & TE)
> +			;

Same comment as above.

> +	}
> +}
> +
> +static int ostm_clock_event_next(unsigned long delta,
> +				     struct clock_event_device *ced)
> +{
> +	struct ostm_device *ostm = ced_to_ostm(ced);
> +
> +	WARN_ON(!clockevent_state_oneshot(ced));

Pointless check. It is up to the time framework to handle that and that is the
case.

> +
> +	ostm_clkevt_timer_stop(ostm);
> +
> +	iowrite32(delta, ostm->base + OSTM_CMP);
> +	iowrite8(CTL_ONESHOT, ostm->base + OSTM_CTL);
> +	iowrite8(TS, ostm->base + OSTM_TS);
> +
> +	return 0;
> +}
> +
> +static int ostm_shutdown(struct clock_event_device *ced)
> +{
> +	struct ostm_device *ostm = ced_to_ostm(ced);
> +
> +	ostm_clkevt_timer_stop(ostm);
> +
> +	return 0;
> +}
> +static int ostm_set_periodic(struct clock_event_device *ced)
> +{
> +	struct ostm_device *ostm = ced_to_ostm(ced);
> +
> +	if (clockevent_state_oneshot(ced) || clockevent_state_periodic(ced))
> +		ostm_clkevt_timer_stop(ostm);
> +
> +	iowrite32(ostm->ticks_per_jiffy - 1, ostm->base + OSTM_CMP);
> +	iowrite8(CTL_PERIODIC, ostm->base + OSTM_CTL);
> +	iowrite8(TS, ostm->base + OSTM_TS);
> +
> +	return 0;
> +}
> +
> +static int ostm_set_oneshot(struct clock_event_device *ced)
> +{
> +	struct ostm_device *ostm = ced_to_ostm(ced);
> +
> +	ostm_clkevt_timer_stop(ostm);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t ostm_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct ostm_device *ostm = dev_id;
> +
> +	if (clockevent_state_oneshot(&ostm->ced))
> +		ostm_clkevt_timer_stop(ostm);
> +
> +	/* notify clockevent layer */
> +	if (ostm->ced.event_handler)
> +		ostm->ced.event_handler(&ostm->ced);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __init ostm_init_clkevt(struct ostm_device *ostm)
> +{
> +	struct clock_event_device *ced = &ostm->ced;
> +	int ret = -ENXIO;
> +
> +	ret = request_irq(ostm->irq, ostm_timer_interrupt,
> +			  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
> +			  dev_name(&ostm->pdev->dev), ostm);

	devm_request_irq

Are you sure the IRQF_NOBALANCING flag should be used ? By default the
interrupt is pinned to cpu0 below. If this timer is used as a broadcast timer
for a system with deep idle, you may want to add the DYNIRQ flag feature to
improve the wakeups on the system.

> +	if (ret) {
> +		dev_err(&ostm->pdev->dev, "failed to request irq\n");
> +		return ret;
> +	}
> +
> +	ced->name = "ostm";
> +	ced->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC;
> +	ced->set_state_shutdown = ostm_shutdown;
> +	ced->set_state_periodic = ostm_set_periodic;
> +	ced->set_state_oneshot = ostm_set_oneshot;
> +	ced->set_next_event = ostm_clock_event_next;
> +	ced->shift = 32;
> +	ced->rating = 300;
> +	ced->cpumask = cpumask_of(0);
> +	clockevents_config_and_register(ced, ostm->rate, 0xf, 0xffffffff);
> +
> +	return 0;
> +}
> +
> +static int __init ostm_probe(struct platform_device *pdev)
> +{
> +	struct ostm_device *ostm;
> +	struct resource *res;
> +	int ret = -EFAULT;
> +
> +	if (!is_early_platform_device(pdev)) {
> +		pm_runtime_set_active(&pdev->dev);
> +		pm_runtime_enable(&pdev->dev);
> +	}

Can you clarify why the 'early' is needed here ?

I don't see the pm_runtime_get/pm_runtime_put in the corresponding function.

What is the benefit of using pm_runtime in this driver ? Isn't the timer
supposed to be in an always-on power domain ?

> +	ostm = platform_get_drvdata(pdev);
> +	if (ostm) {
> +		dev_info(&pdev->dev, "kept as earlytimer\n");
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	ostm = kzalloc(sizeof(*ostm), GFP_KERNEL);

	devm_kzalloc ?

> +	if (!ostm) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");

A memory failure allocation always triggers a dumpstack (IIRC), so this
error message is pointless.

> +		return -ENOMEM;
> +	}
> +
> +	ostm->pdev = pdev;
> +	platform_set_drvdata(ostm->pdev, ostm);
> +
> +	res = platform_get_resource(ostm->pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		dev_err(&ostm->pdev->dev, "failed to get I/O memory\n");
> +		goto err;
> +	}
> +
> +	ostm->base = ioremap_nocache(res->start, resource_size(res));

	devm_ioremap_nocache ?

> +	if (!ostm->base) {
> +		dev_err(&ostm->pdev->dev, "failed to remap I/O memory\n");
> +		goto err;
> +	}
> +
> +	ostm->irq = platform_get_irq(ostm->pdev, 0);
> +	if (ostm->irq < 0) {
> +		dev_err(&ostm->pdev->dev, "failed to get irq\n");
> +		goto err;
> +	}
> +
> +	ostm->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(ostm->clk)) {
> +		dev_err(&ostm->pdev->dev, "failed to get clock\n");
> +		ostm->clk = NULL;
> +		goto err;
> +	}
> +
> +	ret = clk_prepare_enable(ostm->clk);
> +	if (ret) {
> +		dev_err(&ostm->pdev->dev, "failed to enable clock\n");
> +		goto err;
> +	}
> +
> +	ostm->rate = clk_get_rate(ostm->clk);
> +	ostm->ticks_per_jiffy = (ostm->rate + HZ / 2) / HZ;
> +
> +	/* First probed device will be used as system clocksource */
> +	if (!system_clock) {
> +		/* use as clocksource */
> +		ret = ostm_init_clksrc(ostm);
> +
> +		/* use as system scheduling clock */
> +		if (!ret)
> +			ret = ostm_init_sched_clock(ostm);
> +
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to use as sched_clock\n");
> +			system_clock = (void *)-1; /* prevent future attempts */
> +			ret = 0;	/* still works as clocksource */
> +		}

This error code check is unnecessary complex. ostm_init_sched_clock always
return zero.

	if (!system_clock) {
		system_clock = ostm_init_sched_clock(ostm);
		ret = ostm_init_clksrc(ostm)
		if (ret)
			dev_err("Failed to initialize the clocksource\n");
	} else {

		...
	}

This is not perfect but until I send the clksrc_of/clkevt_of split changes, we
have to deal with these kind of hacks.

> +		if (!ret)
> +			dev_info(&pdev->dev, "used for clocksource\n");
> +	} else {
> +		/* use as clock event */
> +		ret = ostm_init_clkevt(ostm);
> +
> +		if (!ret)
> +			dev_info(&pdev->dev, "used for clock events\n");
> +	}
> +
> +err:
> +	if (ret) {
> +		if (ostm->clk)
> +			clk_disable_unprepare(ostm->clk);
> +		if (ostm->base)
> +			iounmap(ostm->base);
> +		kfree(ostm);
> +		platform_set_drvdata(pdev, NULL);

As iomap, kzalloc were done with the devm, then the rollback will be handled
with the device framework.

> +		pm_runtime_idle(&pdev->dev);
> +		return ret;
> +	}
> +
> +	if (is_early_platform_device(pdev))
> +		return ret;
> +
> +out:
> +	pm_runtime_irq_safe(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static int ostm_remove(struct platform_device *pdev)
> +{
> +	return -EBUSY;	/* cannot unregister clockevent */
> +}
> +
> +static const struct of_device_id ostm_of_table[] __maybe_unused = {
> +	{ .compatible = "renesas,ostm" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, ostm_of_table);
> +
> +static struct platform_driver ostm_timer = {
> +	.probe		= ostm_probe,
> +	.remove		= ostm_remove,
> +	.driver	= {
> +		.name	= "ostm",
> +		.of_match_table = of_match_ptr(ostm_of_table),
> +	},
> +};
> +
> +static int __init ostm_init(void)
> +{
> +	return platform_driver_register(&ostm_timer);
> +}
> +
> +static void __exit ostm_exit(void)
> +{
> +	platform_driver_unregister(&ostm_timer);
> +}
> +
> +early_platform_init("earlytimer", &ostm_timer);
> +subsys_initcall(ostm_init);
> +module_exit(ostm_exit);
> +
> +MODULE_AUTHOR("Chris Brandt");
> +MODULE_DESCRIPTION("Renesas OSTM Timer Driver");
> +MODULE_LICENSE("GPL v2");

Maybe you can try with builtin_platform ?

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 0/4] Add support for es8388 and hdmi audio on the rock2
From: Mark Brown @ 2017-01-23 17:54 UTC (permalink / raw)
  To: Romain Perier
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <20170123104149.2508-1-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Mon, Jan 23, 2017 at 11:41:45AM +0100, Romain Perier wrote:

> Changes in v4:
>  - Add support for multi codecs in the asoc machine driver, so the driver
>    matches the hw architecture (analog and hdmi audio are connected on
>    the same i2s line)
>  - Renamed the driver to rk3288-hdmi-analog.c and changed its
>    documentation
>  - Add built-in support for hdmi audio in this driver
>  - Add support for the property 'rockchip,routing'

This is the first version that has been sumbitted upstream.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH v5 2/3] mmc: sh_mobile_sdhi: explain clock bindings
From: Chris Brandt @ 2017-01-23 17:56 UTC (permalink / raw)
  To: Rob Herring
  Cc: Ulf Hansson, Mark Rutland, Simon Horman, Wolfram Sang,
	Geert Uytterhoeven, devicetree@vger.kernel.org,
	linux-mmc@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <20170123165523.fnavkx263ifvss4p@rob-hp-laptop>

Hello Rob,


On Monday, January 23, 2017, Rob Herring wrote:
> > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt
> > @@ -25,8 +25,32 @@ Required properties:
> >  		"renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC
> >  		"renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC
> >
> > +- clocks: Most controllers only have 1 clock source per channel.
> However, on
> > +	  some variations of this controller, the internal card detection
> 
> This should be explicit as to which compatible strings have 2 clocks and
> which have 1 clock.

OK. I'll make a list like I did for sh_mmcif:

https://patchwork.kernel.org/patch/9512091/



> > +
> > +Example showing 2 clocks:
> > +	sdhi0: sd@e804e000 {
> 
> mmc@...

I'm confused. I see that for all SDHI controllers, it either "sd@" or "sdhci@".

$ grep sdhi $(find arch/arm/boot/dts -name "*.dtsi")

$ grep sdhci $(find arch/arm/boot/dts -name "*.dtsi")


Regards,
Chris

^ permalink raw reply

* Re: [PATCH v4 0/9] Implement clocksource for rockchip SoC using rockchip timer
From: Daniel Lezcano @ 2017-01-23 17:57 UTC (permalink / raw)
  To: Alexander Kochetkov
  Cc: Heiko Stübner, LKML, devicetree, LAK, linux-rockchip,
	Thomas Gleixner, Mark Rutland, Rob Herring, Russell King,
	Caesar Wang, Huang Tao
In-Reply-To: <58B283B0-02F6-4342-A6D2-7E082E04863E@gmail.com>

On Mon, Jan 23, 2017 at 08:24:47PM +0300, Alexander Kochetkov wrote:
> Heiko, Daniel, thanks a lot for review!
> I’ll send v5 series this week.
> 
> Heiko, Daniel, may I add 'Reviewed-by:’ to patch series?
> 
> Reviewed-by: Heiko Stübner <heiko@sntech.de> ?
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org> ?
> 

I will answer with the corresponding tag myself when you send the v5.

Thanks.

  -- Daniel

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

^ permalink raw reply

* Re: [PATCH v4 2/4] ASoC: es8328: Add support for slave mode
From: Mark Brown @ 2017-01-23 17:59 UTC (permalink / raw)
  To: Romain Perier
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <20170123104149.2508-3-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

On Mon, Jan 23, 2017 at 11:41:47AM +0100, Romain Perier wrote:

> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +		case SND_SOC_DAIFMT_CBM_CFM:
> +			master = true;
> +			break;
> +		case SND_SOC_DAIFMT_CBS_CFS:
> +			master = false;
> +			break;
> +		default:
> +			return -EINVAL;
> +	}

Please use the normal kernel coding style.  People read in part by
pattern matching so this is important to ease review and coding style
problems are a big warning sign that the code also has problems with
other, more substantial, understanding of how the kernel is expected to
work.

> -	/* Master serial port mode, with BCLK generated automatically */
> -	snd_soc_update_bits(codec, ES8328_MASTERMODE,
> -			ES8328_MASTERMODE_MSC, ES8328_MASTERMODE_MSC);
> +	if (master) {
> +		/* Master serial port mode, with BCLK generated automatically */
> +		snd_soc_update_bits(codec, ES8328_MASTERMODE,
> +				    ES8328_MASTERMODE_MSC,
> +				    ES8328_MASTERMODE_MSC);
> +	} else {
> +		/* Slave serial port mode */
> +		snd_soc_update_bits(codec, ES8328_MASTERMODE,
> +				    ES8328_MASTERMODE_MSC,
> +				    0);
> +	}

Why not just directly do this in the switch statement?  This appears to
be the only place where we change behaviour so setting a variable at the
top of the function then using it at the bottom doesn't seem to add
anything.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v3 5/5] drm/rockchip/dsi: add dw-mipi power domain support
From: Sean Paul @ 2017-01-23 18:03 UTC (permalink / raw)
  To: Chris Zhong
  Cc: mark.rutland, devicetree, pawel.moll, yzq, linux-kernel, dianders,
	dri-devel, tfiga, linux-rockchip, robh+dt, galak,
	linux-arm-kernel
In-Reply-To: <1484907051-7159-6-git-send-email-zyw@rock-chips.com>

On Fri, Jan 20, 2017 at 06:10:51PM +0800, Chris Zhong wrote:
> Reference the power domain incase dw-mipi power down when
> in use.
> 

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
> Changes in v3: None
> 
>  drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 6f0e252..1462101e 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -12,6 +12,7 @@
>  #include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/mfd/syscon.h>
> @@ -293,6 +294,7 @@ struct dw_mipi_dsi {
>  	struct clk *pclk;
>  	struct clk *phy_cfg_clk;
>  
> +	int dpms_mode;
>  	unsigned int lane_mbps; /* per lane */
>  	u32 channel;
>  	u32 lanes;
> @@ -969,6 +971,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  {
>  	struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
>  
> +	if (dsi->dpms_mode != DRM_MODE_DPMS_ON)
> +		return;
> +
>  	if (clk_prepare_enable(dsi->pclk)) {
>  		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>  		return;
> @@ -980,7 +985,9 @@ static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
>  	drm_panel_unprepare(dsi->panel);
>  
>  	dw_mipi_dsi_disable(dsi);
> +	pm_runtime_put(dsi->dev);
>  	clk_disable_unprepare(dsi->pclk);
> +	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
>  }
>  
>  static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> @@ -990,11 +997,15 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  	int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
>  	u32 val;
>  
> +	if (dsi->dpms_mode == DRM_MODE_DPMS_ON)
> +		return;
> +
>  	if (clk_prepare_enable(dsi->pclk)) {
>  		dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
>  		return;
>  	}
>  
> +	pm_runtime_get_sync(dsi->dev);
>  	dw_mipi_dsi_init(dsi);
>  	dw_mipi_dsi_dpi_config(dsi);
>  	dw_mipi_dsi_packet_handler_config(dsi);
> @@ -1030,6 +1041,7 @@ static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
>  
>  	regmap_write(dsi->grf_regmap, pdata->grf_switch_reg, val);
>  	dev_dbg(dsi->dev, "vop %s output to dsi0\n", (mux) ? "LIT" : "BIG");
> +	dsi->dpms_mode = DRM_MODE_DPMS_ON;
>  }
>  
>  static int
> @@ -1198,6 +1210,7 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  
>  	dsi->dev = dev;
>  	dsi->pdata = pdata;
> +	dsi->dpms_mode = DRM_MODE_DPMS_OFF;
>  
>  	ret = rockchip_mipi_parse_dt(dsi);
>  	if (ret)
> @@ -1271,6 +1284,8 @@ static int dw_mipi_dsi_bind(struct device *dev, struct device *master,
>  
>  	dev_set_drvdata(dev, dsi);
>  
> +	pm_runtime_enable(dev);
> +
>  	dsi->dsi_host.ops = &dw_mipi_dsi_host_ops;
>  	dsi->dsi_host.dev = dev;
>  	ret = mipi_dsi_host_register(&dsi->dsi_host);
> @@ -1293,6 +1308,7 @@ static void dw_mipi_dsi_unbind(struct device *dev, struct device *master,
>  	struct dw_mipi_dsi *dsi = dev_get_drvdata(dev);
>  
>  	mipi_dsi_host_unregister(&dsi->dsi_host);
> +	pm_runtime_disable(dev);
>  	clk_disable_unprepare(dsi->pllref_clk);
>  }
>  
> -- 
> 2.6.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH v4 3/4] ASoC: rockchip: Add machine driver for RK3288 boards that use analog/HDMI
From: Mark Brown @ 2017-01-23 18:08 UTC (permalink / raw)
  To: Romain Perier
  Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Heiko Stuebner,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <20170123104149.2508-4-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

On Mon, Jan 23, 2017 at 11:41:48AM +0100, Romain Perier wrote:

This looks mostly fine but:

> +	machine->gpio_hp_det = of_get_named_gpio(np,
> +		"rockchip,hp-det-gpios", 0);
> +	if (machine->gpio_hp_det == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;

Rather than explicitly checking for probe defer here it's better to
check for the error code that indicates there's nothing to find (-ENODEV
IIRC but I might be misremembering for GPIO).  That way if there's some
actual error like running out of memory allocating some structure that's
required then we'll not just silently ignore it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [alsa-devel] [RFC][PATCH] ASoC: add simple-graph-card document
From: Sylwester Nawrocki @ 2017-01-23 18:14 UTC (permalink / raw)
  To: Kuninori Morimoto, Rob Herring; +Cc: Mark Brown, Linux-DT, Linux-ALSA
In-Reply-To: <87tw8q1bct.wl%kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>


Hi Morimoto-san,

On 01/23/2017 08:33 AM, Kuninori Morimoto wrote:
> Hi Rob, Mark
> 
> I created OF-graph base simple sound card.
> I want to post this patch-set, but it depends many things.
> Thus, before posting patch-set, I would like to know
> OF-graph sound DT binding was OK or not.
> Can you give me comments ?
> 
> ---
>  .../bindings/sound/simple-graph-card.txt           | 140 +++++++++++++++++++++
>  1 file changed, 140 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/simple-graph-card.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/simple-graph-card.txt 
> b/Documentation/devicetree/bindings/sound/simple-graph-card.txt
> new file mode 100644
> index 0000000..4b29284
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/simple-graph-card.txt
> @@ -0,0 +1,140 @@
> +Simple-Graph-Card:

This binding is likely going to cover most of existing sound system
configuration, at least it looks as it has a potential to be scalable
enough.  So perhaps we could treat is as a more generic binding and
could drop the "Simple" word now?  I'm not 100% sure it's a good idea,
but maybe we could make it "Generic Sound Card DT Binding"?

> +Simple-Graph-Card specifies audio DAI connections of SoC <-> codec.
> +It is based on common bindings for device graphs.
> +see ${LINUX}/Documentation/devicetree/bindings/graph.txt

The DT bindings should be OS agnostic, I think we should drop the 
"${LINUX}/" part here and elsewhere in this documentation.

> +Basically, Simple-Graph-Card property is same as Simple-Card.
> +see ${LINUX}/Documentation/devicetree/bindings/sound/simple-card.txt
> +
> +Below are same as Simple-Card.
> +
> +- simple-audio-card,name
> +- simple-audio-card,format
> +- simple-audio-card,frame-master
> +- simple-audio-card,bitclock-master
> +- simple-audio-card,bitclock-inversion
> +- simple-audio-card,frame-inversion
> +- simple-audio-card,dai-tdm-slot-num
> +- simple-audio-card,dai-tdm-slot-width
> +- clocks / system-clock-frequency
> +
> +These should be implemented
> +- simple-audio-card,widgets
> +- simple-audio-card,routing
> +- simple-audio-card,mclk-fs
> +- simple-audio-card,hp-det-gpio
> +- simple-audio-card,mic-det-gpio
> +
> +Required properties:
> +
> +- compatible				: "asoc-simple-graph-card";

"asoc" is Linux related, we shouldn't be putting this in the compatible string.
Perhaps "generic-sound-card" ? Again, I'm sure if it is expected to have such
a generic kind of DT binding.

> +Optional properties:
> +
> +- dais				: Sound DAI phandle list if Card has
> +					  multi DAI. see Example
> +
> +Each ports
> +
> +- port@0				: CPU setting
> +- port@1				: Codec setting
> +
> +Example: Single DAI case
> +
> +	sound_card: sound {
> +		compatible = "asoc-simple-graph-card";
> +
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				sound0_in: endpoint {
> +					remote-endpoint = <&rsnd_port0>;
> +
> +					simple-audio-card,format = "left_j";
> +					simple-audio-card,bitclock-master = <&rsnd_port0>;
> +					simple-audio-card,frame-master = <&rsnd_port0>;
> +				};
> +			};
> +			port@1 {
> +				sound0_out: endpoint {
> +					remote-endpoint = <&ak4613_port>;
> +				};
> +			};
> +		};
> +	};

I wouldn't be making a separate case for single DAI. The 'ports' node can 
be omitted, port@0, port@1 nodes could be put under respective device nodes 
and in the 'sound' node we would have 'dais' property pointing to the CPU 
DAI port,  not the endpoint.  The endpoint is supposed to describe one of 
possible configurations of the port.  I think we want phandles to the 'port' 
nodes, not phandles to the 'endpoint' nodes in the 'sound' node.

> +Example: Multi DAI case
> +
> +	sound_dai0: sound_dai@0 {
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>; 
> +			port@0 {
> +				sound0_in: endpoint {
> +					remote-endpoint = <&rsnd_port0>;
> +
> +					simple-audio-card,format = "left_j";
> +					simple-audio-card,bitclock-master = <&rsnd_port0>;
> +					simple-audio-card,frame-master = <&rsnd_port0>;
> +				};
> +			};
> +			port@1 {
> +				sound0_out: endpoint {
> +					remote-endpoint = <&ak4613_port>;
> +				};
> +			};
> +		};
> +	};
> +
> +	sound_dai1: sound_dai@1 {
> +		ports {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			port@0 {
> +				sound1_in: endpoint {
> +					remote-endpoint = <&rsnd_port1>;
> +
> +					simple-audio-card,format = "i2s";
> +					simple-audio-card,bitclock-master = <&rsnd_port1>;
> +					simple-audio-card,frame-master = <&rsnd_port1>;
> +				};
> +			};
> +			port@1 {
> +				sound1_out: endpoint {
> +					remote-endpoint = <&rcar_dw_hdmi0_snd_in>;
> +				};
> +			};
> +		};
> +	};

> +
> +	rsnd_ak4613: sound {
> +		compatible = "asoc-simple-graph-card";
> +
> +		dais = <&sound_dai0
> +			&sound_dai1
> +			&sound_dai2>;
> +	};

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

^ permalink raw reply

* Re: [PATCH] net: ethernet: mvneta: add support for 2.5G DRSGMII mode
From: Andrew Lunn @ 2017-01-23 18:18 UTC (permalink / raw)
  To: Jan Luebbe
  Cc: netdev, devicetree, davem, Rob Herring, Mark Rutland,
	Thomas Petazzoni, Florian Fainelli
In-Reply-To: <20170123142206.5390-1-jlu@pengutronix.de>

On Mon, Jan 23, 2017 at 03:22:06PM +0100, Jan Luebbe wrote:
> The Marvell MVNETA Ethernet controller supports a 2.5 Gbps SGMII mode
> called DRSGMII.
> 
> This patch adds a corresponding phy-mode string 'drsgmii' and parses it
> from DT. The MVNETA then configures the SERDES protocol value
> accordingly.

Hi Jan

So this sets the PHY to 2.5Gbps. What about the MAC? Currently,
mvneta_adjust_link() knows about SPEED_1000 and SPEED_100. Does it now
need to know about SPEED_2500? mvneta_ethtool_set_link_ksettings()?

Thanks
	Andrew

^ permalink raw reply

* Re: [PATCH v2 0/9] ARM: dts: support I2SE Duckbill devices
From: Fabio Estevam @ 2017-01-23 18:18 UTC (permalink / raw)
  To: Michael Heimpold
  Cc: Shawn Guo, Sascha Hauer,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Mark Rutland,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Stefan Wahren
In-Reply-To: <1485123764-14754-1-git-send-email-mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org>

Hi Michael,

On Sun, Jan 22, 2017 at 8:22 PM, Michael Heimpold <mhei-Z/Lg1yOAjpkb1SvskN2V4Q@public.gmane.org> wrote:

> - Duckbill 485/Duckbill 2 485: as the name implies, these
>   devices are intended to be used as Ethernet - RS485 converters

Just curious: do you use mxs-auart to interface to a rs485
transceiver? If so, would you have mxs-auart patches that add rs485?

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

^ permalink raw reply

* Re: [PATCH 3/3] ARM64: dts: meson-gxm-rbox-pro: Add board compatible string
From: Kevin Hilman @ 2017-01-23 18:21 UTC (permalink / raw)
  To: Andreas Färber
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Carlo Caione,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ada-MUr+2Ic57fgS+FvcfC7Uqw,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170117225459.4438-3-afaerber-l3A5Bk7waGM@public.gmane.org>

Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org> writes:

> Cc: ada-MUr+2Ic57fgS+FvcfC7Uqw@public.gmane.org
> Signed-off-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
> ---
>  To be squashed in v2.

FYI, since you mention v2 here, i'll wait on this series and assume
you'll update it on top of your cleanup series.

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

^ permalink raw reply

* Re: [PATCHv2] Documentation: dt-bindings: Add binding documentation for TI clkctrl clocks
From: Tero Kristo @ 2017-01-23 18:29 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Michael Turquette, Stephen Boyd,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Paul Walmsley, Rob Herring
In-Reply-To: <20170123162828.GR7403-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

On 23/01/17 18:28, Tony Lindgren wrote:
> * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [170123 06:45]:
>> On 18/01/17 00:53, Tony Lindgren wrote:
>>> Texas Instruments omap variant SoCs starting with omap4 have a clkctrl
>>> clock controller instance for each interconnect target module. The clkctrl
>>> controls functional and interface clocks for the module.
>>>
>>> The clkctrl clocks are currently handled by arch/arm/mach-omap2 hwmod code.
>>> With this binding and a related clock device driver we can start moving the
>>> clkctrl clock handling to live in drivers/clk/ti.
>>>
>>> Note that this binding allows keeping the clockdomain related parts out of
>>> drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
>>> a separate driver in drivers/soc/ti and genpd. If the clockdomain driver
>>> needs to know it's clocks, we can just set the the clkctrl device
>>> instances to be children of the related clockdomain device.
>>>
>>> Each clkctrl clock can have multiple optional gate clocks, and multiple
>>> optional mux clocks. To represent this in device tree, it seems that
>>> it is best done using four clock cells #clock-cells = <4> property.
>>>
>>> The reasons for using #clock-cells = <4> are:
>>>
>>> 1. We need to specify the clkctrl offset from the instance base. Otherwise
>>>    we end up with a large number of device tree nodes that need to be
>>>    patched when new clocks are discovered in a clkctrl clock with minor
>>>    hardware revision changes for example
>>>
>>> 2. On omap5 CM_L3INIT_USB_HOST_HS_CLKCTRL has ten OPTFCLKEN bits. So we
>>>    need to use a separate cell for optional gate clocks to avoid address
>>>    space conflicts
>>>
>>> 3. Some clkctrl instances can also also optional mux clocks. To address
>>>    them properly we need also a separate cell for the optional mux
>>>    clock index
>>>
>>> 4. The modulemode clock needs a flag passed to it for hardware or
>>>    software controlled mode
>>
>> Hi Tony,
>>
>> I think #clock-cells = <4> is too much. I believe we only need 2:
>>
>> - one entry for clkctrl offset
>> - one entry for clock offset within the clkctrl entry (0 = module clock, 8+
>> = opt-clocks / mux clocks / dividers)
>
> OK the less #clock-cells the better as long as it's enough :)
>
>> Fields 2 / 3 in your proposal are mutually exclusive, if either field is
>> non-zero, the other one must be zero. And, the opt clocks / mux / divs
>> always have different values for these.
>
> OK. Just to confirm the assumptions then:
>
> 1. The optional mux clock the consumer needs to select the right
>    source clock with with clk_set_parent()

Yes. And for this you need to fetch a clock handle via some mechanism 
(of_clk_get, clk_get...) Clock consumers can't directly use parent IDs.

>
> 2. The optional divider clock rate must be set by the consumer
>    using clk_set_rate()

Yes again.

>
> And in that case we again don't need to define any artificial
> clock indexes, which is good if new clocks are discovered between
> various SoC revisions.
>
>> Field 4 is kind of redundant also, as the module clock must be registered at
>> the clkctrl probe time, it is too late for the clock consumer to provide the
>> proper setting for the clock during its own probe. It seems I need to add
>> static data to driver which basically has this information in place already.
>
> OK yeah good point, the "clocks" is a consumer property.
>
> So in that case we must also assume that if any clock consumer needs
> to change between HWSUP or SWSUP, it needs to be done with some yet
> to be determined API. We have not needed that so far AFAIK though.
>
> If there are no issues with the above, I'm naturally fine using the
> #clock-cells = <2> :)

Yeah, clock-cells = <2>; seems to work just fine in the WIP codebase I have.

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

^ permalink raw reply

* Re: [PATCH 3/3] ARM64: dts: meson-gxm-rbox-pro: Add board compatible string
From: Andreas Färber @ 2017-01-23 18:32 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Carlo Caione,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, ada-MUr+2Ic57fgS+FvcfC7Uqw,
	Rob Herring, Mark Rutland, Catalin Marinas, Will Deacon,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <m2h94p3aee.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>

Am 23.01.2017 um 19:21 schrieb Kevin Hilman:
> Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org> writes:
> 
>> Cc: ada-MUr+2Ic57fgS+FvcfC7Uqw@public.gmane.org
>> Signed-off-by: Andreas Färber <afaerber-l3A5Bk7waGM@public.gmane.org>
>> ---
>>  To be squashed in v2.
> 
> FYI, since you mention v2 here, i'll wait on this series and assume
> you'll update it on top of your cleanup series.

Yes, I am waiting on the board documentation cleanup to be merged before
I rebase though. The others shouldn't matter here.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 1/2] dt-bindings: pwm: Add MediaTek PWM bindings
From: John Crispin @ 2017-01-23 18:34 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring, Matthias Brugger
  Cc: linux-pwm, devicetree, linux-mediatek,
	Sean Wang (王志亘), John Crispin

Document the device-tree binding of MediaTek PWM. The PWM has 5 channels.
This has been tested on MT7623 only but should work on all the other MTK
SoCs that contain this core.

Signed-off-by: John Crispin <john@phrozen.org>
---
 .../devicetree/bindings/pwm/pwm-mediatek.txt       |   34 ++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-mediatek.txt

diff --git a/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
new file mode 100644
index 0000000..54c59b0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-mediatek.txt
@@ -0,0 +1,34 @@
+MediaTek PWM controller
+
+Required properties:
+ - compatible: should be "mediatek,<name>-pwm":
+   - "mediatek,mt7623-pwm": found on mt7623 SoC.
+ - reg: physical base address and length of the controller's registers.
+ - #pwm-cells: must be 2. See pwm.txt in this directory for a description of
+   the cell format.
+ - clocks: phandle and clock specifier of the PWM reference clock.
+ - clock-names: must contain the following:
+   - "top": the top clock generator
+   - "main": clock used by the PWM core
+   - "pwm1-5": the five per PWM clocks
+ - pinctrl-names: Must contain a "default" entry.
+ - pinctrl-0: One property must exist for each entry in pinctrl-names.
+   See pinctrl/pinctrl-bindings.txt for details of the property values.
+
+Example:
+	pwm0: pwm@11006000 {
+		compatible = "mediatek,mt7623-pwm";
+		reg = <0 0x11006000 0 0x1000>;
+		#pwm-cells = <2>;
+		clocks = <&topckgen CLK_TOP_PWM_SEL>,
+			 <&pericfg CLK_PERI_PWM>,
+			 <&pericfg CLK_PERI_PWM1>,
+			 <&pericfg CLK_PERI_PWM2>,
+			 <&pericfg CLK_PERI_PWM3>,
+			 <&pericfg CLK_PERI_PWM4>,
+			 <&pericfg CLK_PERI_PWM5>;
+		clock-names = "top", "main", "pwm1", "pwm2",
+			      "pwm3", "pwm4", "pwm5";
+		pinctrl-names = "default";
+		pinctrl-0 = <&pwm0_pins>;
+	};
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/2] pwm: add pwm-mediatek
From: John Crispin @ 2017-01-23 18:34 UTC (permalink / raw)
  To: Thierry Reding, Rob Herring, Matthias Brugger
  Cc: linux-pwm, devicetree, linux-mediatek,
	Sean Wang (王志亘), John Crispin
In-Reply-To: <1485196477-669-1-git-send-email-john@phrozen.org>

This patch adds support for the PWM core found on current ARM base SoCs
made by MediaTek. This IP core supports 5 channels and has 2 operational
modes. There is the old mode, which is a classical PWM and the new mode
which allows the user to define bitmasks that get clocked out on the pins.
As the subsystem currently only supports PWM cores with the "old" mode,
we can safely ignore the "new" mode for now.

Signed-off-by: John Crispin <john@phrozen.org>
---
 drivers/pwm/Kconfig        |    9 ++
 drivers/pwm/Makefile       |    1 +
 drivers/pwm/pwm-mediatek.c |  223 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 233 insertions(+)
 create mode 100644 drivers/pwm/pwm-mediatek.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index f92dd41..dd09e2f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -291,6 +291,15 @@ config PWM_MTK_DISP
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-mtk-disp.
 
+config PWM_MEDIATEK
+	tristate "MediaTek PWM support"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  Generic PWM framework driver for Mediatek ARM SoC.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-mxs.
+
 config PWM_MXS
 	tristate "Freescale MXS PWM support"
 	depends on ARCH_MXS && OF
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index a48bdb5..f897f086 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_PWM_LPSS)		+= pwm-lpss.o
 obj-$(CONFIG_PWM_LPSS_PCI)	+= pwm-lpss-pci.o
 obj-$(CONFIG_PWM_LPSS_PLATFORM)	+= pwm-lpss-platform.o
 obj-$(CONFIG_PWM_MESON)		+= pwm-meson.o
+obj-$(CONFIG_PWM_MEDIATEK)	+= pwm-mediatek.o
 obj-$(CONFIG_PWM_MTK_DISP)	+= pwm-mtk-disp.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_OMAP_DMTIMER)	+= pwm-omap-dmtimer.o
diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
new file mode 100644
index 0000000..dfb9834
--- /dev/null
+++ b/drivers/pwm/pwm-mediatek.c
@@ -0,0 +1,223 @@
+/*
+ * Mediatek Pulse Width Modulator driver
+ *
+ * Copyright (C) 2015 John Crispin <blogic@openwrt.org>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+/* the IP supports 5 PWM channels */
+#define MTK_NUM_PWM		5
+
+/* PWM registers and bits definitions */
+#define PWMCON			0x00
+#define PWMHDUR			0x04
+#define PWMLDUR			0x08
+#define PWMGDUR			0x0c
+#define PWMWAVENUM		0x28
+#define PWMDWIDTH		0x2c
+#define PWMTHRES		0x30
+
+enum {
+	MTK_CLK_MAIN = 0,
+	MTK_CLK_TOP,
+	MTK_CLK_PWM1,
+	MTK_CLK_PWM2,
+	MTK_CLK_PWM3,
+	MTK_CLK_PWM4,
+	MTK_CLK_PWM5,
+	MTK_CLK_MAX,
+};
+
+static const char * const mtk_pwm_clk_name[] = {
+	"main", "top", "pwm1", "pwm2", "pwm3", "pwm4", "pwm5"
+};
+
+/**
+ * struct mtk_pwm_chip - struct representing pwm chip
+ *
+ * @mmio_base: base address of pwm chip
+ * @chip: linux pwm chip representation
+ */
+struct mtk_pwm_chip {
+	void __iomem *mmio_base;
+	struct pwm_chip chip;
+	struct clk *clks[MTK_CLK_MAX];
+};
+
+static inline struct mtk_pwm_chip *to_mtk_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct mtk_pwm_chip, chip);
+}
+
+static inline u32 mtk_pwm_readl(struct mtk_pwm_chip *chip, unsigned int num,
+				  unsigned long offset)
+{
+	return ioread32(chip->mmio_base + 0x10 + (num * 0x40) + offset);
+}
+
+static inline void mtk_pwm_writel(struct mtk_pwm_chip *chip,
+				    unsigned int num, unsigned long offset,
+				    unsigned long val)
+{
+	iowrite32(val, chip->mmio_base + 0x10 + (num * 0x40) + offset);
+}
+
+static int mtk_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			    int duty_ns, int period_ns)
+{
+	struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+	u32 resolution = 100 / 4;
+	u32 clkdiv = 0;
+
+	resolution = 1000000000 /
+		     (clk_get_rate(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]));
+
+	while (period_ns / resolution  > 8191) {
+		clkdiv++;
+		resolution *= 2;
+	}
+
+	if (clkdiv > 7)
+		return -1;
+
+	mtk_pwm_writel(pc, pwm->hwpwm, PWMCON, BIT(15) | BIT(3) | clkdiv);
+	mtk_pwm_writel(pc, pwm->hwpwm, PWMDWIDTH, period_ns / resolution);
+	mtk_pwm_writel(pc, pwm->hwpwm, PWMTHRES, duty_ns / resolution);
+	return 0;
+}
+
+static int mtk_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+	u32 val;
+	int ret;
+
+	ret = clk_prepare(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
+	if (ret < 0)
+		return ret;
+
+	val = ioread32(pc->mmio_base);
+	val |= BIT(pwm->hwpwm);
+	iowrite32(val, pc->mmio_base);
+
+	return 0;
+}
+
+static void mtk_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct mtk_pwm_chip *pc = to_mtk_pwm_chip(chip);
+	u32 val;
+
+	val = ioread32(pc->mmio_base);
+	val &= ~BIT(pwm->hwpwm);
+	iowrite32(val, pc->mmio_base);
+	clk_unprepare(pc->clks[MTK_CLK_PWM1 + pwm->hwpwm]);
+}
+
+static const struct pwm_ops mtk_pwm_ops = {
+	.config = mtk_pwm_config,
+	.enable = mtk_pwm_enable,
+	.disable = mtk_pwm_disable,
+	.owner = THIS_MODULE,
+};
+
+static int mtk_pwm_probe(struct platform_device *pdev)
+{
+	struct mtk_pwm_chip *pc;
+	struct resource *r;
+	int ret, i;
+
+	pc = devm_kzalloc(&pdev->dev, sizeof(*pc), GFP_KERNEL);
+	if (!pc)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	pc->mmio_base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(pc->mmio_base))
+		return PTR_ERR(pc->mmio_base);
+
+	for (i = 0; i < MTK_CLK_MAX; i++) {
+		pc->clks[i] = devm_clk_get(&pdev->dev, mtk_pwm_clk_name[i]);
+		if (IS_ERR(pc->clks[i]))
+			return PTR_ERR(pc->clks[i]);
+	}
+
+	ret = clk_prepare(pc->clks[MTK_CLK_TOP]);
+	if (ret < 0)
+		return ret;
+
+	ret = clk_prepare(pc->clks[MTK_CLK_MAIN]);
+	if (ret < 0)
+		goto disable_clk_top;
+
+	platform_set_drvdata(pdev, pc);
+
+	pc->chip.dev = &pdev->dev;
+	pc->chip.ops = &mtk_pwm_ops;
+	pc->chip.base = -1;
+	pc->chip.npwm = MTK_NUM_PWM;
+
+	ret = pwmchip_add(&pc->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
+		goto disable_clk_main;
+	}
+
+	return 0;
+
+disable_clk_main:
+	clk_unprepare(pc->clks[MTK_CLK_MAIN]);
+disable_clk_top:
+	clk_unprepare(pc->clks[MTK_CLK_TOP]);
+
+	return ret;
+}
+
+static int mtk_pwm_remove(struct platform_device *pdev)
+{
+	struct mtk_pwm_chip *pc = platform_get_drvdata(pdev);
+	int i;
+
+	for (i = 0; i < MTK_NUM_PWM; i++)
+		pwm_disable(&pc->chip.pwms[i]);
+
+	return pwmchip_remove(&pc->chip);
+}
+
+static const struct of_device_id mtk_pwm_of_match[] = {
+	{ .compatible = "mediatek,mt7623-pwm" },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(of, mtk_pwm_of_match);
+
+static struct platform_driver mtk_pwm_driver = {
+	.driver = {
+		.name = "mtk-pwm",
+		.owner = THIS_MODULE,
+		.of_match_table = mtk_pwm_of_match,
+	},
+	.probe = mtk_pwm_probe,
+	.remove = mtk_pwm_remove,
+};
+
+module_platform_driver(mtk_pwm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("John Crispin <blogic@openwrt.org>");
+MODULE_ALIAS("platform:mtk-pwm");
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH 01/18] Documentation: devicetree: amlogic: Reorder boards
From: Andreas Färber @ 2017-01-23 18:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-amlogic, Carlo Caione, Kevin Hilman, linux-arm-kernel,
	linux-kernel, Mark Rutland, devicetree
In-Reply-To: <20170123164831.ux2kkofq7xf2whnp@rob-hp-laptop>

Am 23.01.2017 um 17:48 schrieb Rob Herring:
> On Fri, Jan 20, 2017 at 11:21:01PM +0100, Andreas Färber wrote:
>> Enforce groupment by SoCs and order alphabetically within the group
>> (with some exceptions). This should facilitate adding new boards.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  Documentation/devicetree/bindings/arm/amlogic.txt | 19 +++++++++++++------
>>  1 file changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/amlogic.txt b/Documentation/devicetree/bindings/arm/amlogic.txt
>> index c246cd2730d9..b03d23c9ea78 100644
>> --- a/Documentation/devicetree/bindings/arm/amlogic.txt
>> +++ b/Documentation/devicetree/bindings/arm/amlogic.txt
>> @@ -29,23 +29,30 @@ Boards with the Amlogic Meson GXM S912 SoC shall have the following properties:
>>    Required root node property:
>>      compatible: "amlogic,s912", "amlogic,meson-gxm";
>>  
>> -Board compatible values:
>> +Board compatible values (grouped by SoC):
> 
> ...and alphabetical order within each SoC.

...except for pro, meta, telos, which are in increasing feature set
order - should we reorder them, or ignore but still write alphabetical?

Regards,
Andreas

> 
>> +
>>    - "geniatech,atv1200" (Meson6)
>> +
>>    - "minix,neo-x8" (Meson8)
>> -  - "tronfy,mxq" (Meson8b)
>> +
>>    - "hardkernel,odroid-c1" (Meson8b)
>> +  - "tronfy,mxq" (Meson8b)
>> +
>> +  - "amlogic,p200" (Meson gxbb)
>> +  - "amlogic,p201" (Meson gxbb)
>> +  - "hardkernel,odroid-c2" (Meson gxbb)
>> +  - "nexbox,a95x" (Meson gxbb or Meson gxl s905x)
>>    - "tronsmart,vega-s95-pro", "tronsmart,vega-s95" (Meson gxbb)
>>    - "tronsmart,vega-s95-meta", "tronsmart,vega-s95" (Meson gxbb)
>>    - "tronsmart,vega-s95-telos", "tronsmart,vega-s95" (Meson gxbb)
>> -  - "hardkernel,odroid-c2" (Meson gxbb)
>> -  - "amlogic,p200" (Meson gxbb)
>> -  - "amlogic,p201" (Meson gxbb)
>>    - "wetek,hub" (Meson gxbb)
>>    - "wetek,play2" (Meson gxbb)
>> +
>>    - "amlogic,p212" (Meson gxl s905x)
>> +
>>    - "amlogic,p230" (Meson gxl s905d)
>>    - "amlogic,p231" (Meson gxl s905d)
>> +
>>    - "amlogic,q200" (Meson gxm s912)
>>    - "amlogic,q201" (Meson gxm s912)
>> -  - "nexbox,a95x" (Meson gxbb or Meson gxl s905x)
>>    - "nexbox,a1" (Meson gxm s912)
>> -- 
>> 2.10.2
>>


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply

* Re: [PATCHv2] Documentation: dt-bindings: Add binding documentation for TI clkctrl clocks
From: Tony Lindgren @ 2017-01-23 18:38 UTC (permalink / raw)
  To: Tero Kristo
  Cc: Michael Turquette, Stephen Boyd,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Paul Walmsley, Rob Herring
In-Reply-To: <f3c4154d-09d0-3930-4452-1f7d61622f3d-l0cyMroinI0@public.gmane.org>

* Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [170123 10:30]:
> On 23/01/17 18:28, Tony Lindgren wrote:
> > * Tero Kristo <t-kristo-l0cyMroinI0@public.gmane.org> [170123 06:45]:
> > > On 18/01/17 00:53, Tony Lindgren wrote:
> > > > Texas Instruments omap variant SoCs starting with omap4 have a clkctrl
> > > > clock controller instance for each interconnect target module. The clkctrl
> > > > controls functional and interface clocks for the module.
> > > > 
> > > > The clkctrl clocks are currently handled by arch/arm/mach-omap2 hwmod code.
> > > > With this binding and a related clock device driver we can start moving the
> > > > clkctrl clock handling to live in drivers/clk/ti.
> > > > 
> > > > Note that this binding allows keeping the clockdomain related parts out of
> > > > drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
> > > > a separate driver in drivers/soc/ti and genpd. If the clockdomain driver
> > > > needs to know it's clocks, we can just set the the clkctrl device
> > > > instances to be children of the related clockdomain device.
> > > > 
> > > > Each clkctrl clock can have multiple optional gate clocks, and multiple
> > > > optional mux clocks. To represent this in device tree, it seems that
> > > > it is best done using four clock cells #clock-cells = <4> property.
> > > > 
> > > > The reasons for using #clock-cells = <4> are:
> > > > 
> > > > 1. We need to specify the clkctrl offset from the instance base. Otherwise
> > > >    we end up with a large number of device tree nodes that need to be
> > > >    patched when new clocks are discovered in a clkctrl clock with minor
> > > >    hardware revision changes for example
> > > > 
> > > > 2. On omap5 CM_L3INIT_USB_HOST_HS_CLKCTRL has ten OPTFCLKEN bits. So we
> > > >    need to use a separate cell for optional gate clocks to avoid address
> > > >    space conflicts
> > > > 
> > > > 3. Some clkctrl instances can also also optional mux clocks. To address
> > > >    them properly we need also a separate cell for the optional mux
> > > >    clock index
> > > > 
> > > > 4. The modulemode clock needs a flag passed to it for hardware or
> > > >    software controlled mode
> > > 
> > > Hi Tony,
> > > 
> > > I think #clock-cells = <4> is too much. I believe we only need 2:
> > > 
> > > - one entry for clkctrl offset
> > > - one entry for clock offset within the clkctrl entry (0 = module clock, 8+
> > > = opt-clocks / mux clocks / dividers)
> > 
> > OK the less #clock-cells the better as long as it's enough :)
> > 
> > > Fields 2 / 3 in your proposal are mutually exclusive, if either field is
> > > non-zero, the other one must be zero. And, the opt clocks / mux / divs
> > > always have different values for these.
> > 
> > OK. Just to confirm the assumptions then:
> > 
> > 1. The optional mux clock the consumer needs to select the right
> >    source clock with with clk_set_parent()
> 
> Yes. And for this you need to fetch a clock handle via some mechanism
> (of_clk_get, clk_get...) Clock consumers can't directly use parent IDs.
> 
> > 
> > 2. The optional divider clock rate must be set by the consumer
> >    using clk_set_rate()
> 
> Yes again.
> 
> > 
> > And in that case we again don't need to define any artificial
> > clock indexes, which is good if new clocks are discovered between
> > various SoC revisions.
> > 
> > > Field 4 is kind of redundant also, as the module clock must be registered at
> > > the clkctrl probe time, it is too late for the clock consumer to provide the
> > > proper setting for the clock during its own probe. It seems I need to add
> > > static data to driver which basically has this information in place already.
> > 
> > OK yeah good point, the "clocks" is a consumer property.
> > 
> > So in that case we must also assume that if any clock consumer needs
> > to change between HWSUP or SWSUP, it needs to be done with some yet
> > to be determined API. We have not needed that so far AFAIK though.
> > 
> > If there are no issues with the above, I'm naturally fine using the
> > #clock-cells = <2> :)
> 
> Yeah, clock-cells = <2>; seems to work just fine in the WIP codebase I have.

OK thanks for confirming, will post v3 of the binding.

Regards,

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

^ permalink raw reply


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