Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] pinctrl: core: Fix regression caused by delayed work for hogs
From: Linus Walleij @ 2017-01-13 15:26 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linux-OMAP, Gary Bisson
In-Reply-To: <20170111221334.26194-1-tony@atomide.com>

On Wed, Jan 11, 2017 at 11:13 PM, Tony Lindgren <tony@atomide.com> wrote:

> Commit df61b366af26 ("pinctrl: core: Use delayed work for hogs") caused a
> regression at least with sh-pfc that is also a GPIO controller as
> noted by Geert Uytterhoeven <geert@linux-m68k.org>.
>
> As the original pinctrl_register() has issues calling pin controller
> driver functions early before the controller has finished registering,
> we can't just revert commit df61b366af26. That would break the drivers
> using GENERIC_PINCTRL_GROUPS or GENERIC_PINMUX_FUNCTIONS.
>
> So let's fix the issue with the following steps as a single patch:
>
> 1. Revert the late_init parts of commit df61b366af26.
>
>    The late_init clearly won't work and we have to just give up
>    on fixing pinctrl_register() for GENERIC_PINCTRL_GROUPS and
>    GENERIC_PINMUX_FUNCTIONS.
>
> 2. Split pinctrl_register() into two parts
>
>    By splitting pinctrl_register() into pinctrl_init_controller()
>    and pinctrl_create_and_start() we have better control over when
>    it's safe to call pinctrl_create().
>
> 3. Introduce a new pinctrl_register_and_init() function
>
>    As suggested by Linus Walleij <linus.walleij@linaro.org>, we
>    can just introduce a new function for the controllers that need
>    pinctrl_create() called later.
>
> 4. Convert the four known problem cases to use new function
>
>    Let's convert pinctrl-imx, pinctrl-single, sh-pfc and ti-iodelay
>    to use the new function to fix the issues. The rest of the drivers
>    can be converted later. Let's also update Documentation/pinctrl.txt
>    accordingly because of the known issues with pinctrl_register().
>
> Fixes: df61b366af26 ("pinctrl: core: Use delayed work for hogs")
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Gary Bisson <gary.bisson@boundarydevices.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

OK patch applied.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v2 6/6] arm: dts: mt2701: Add thermal device node.
From: Matthias Brugger @ 2017-01-13 15:27 UTC (permalink / raw)
  To: Erin Lo
  Cc: srv_heupstream, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek, Dawei Chien
In-Reply-To: <1484296978-18572-7-git-send-email-erin.lo@mediatek.com>



On 13/01/17 09:42, Erin Lo wrote:
> From: Dawei Chien <dawei.chien@mediatek.com>
>
> Add thermal controller device nodes for MT2701.
>
> Signed-off-by: Dawei Chien <dawei.chien@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
>  arch/arm/boot/dts/mt2701.dtsi | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index 3847f70..c43d5f8 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -89,6 +89,36 @@
>  		clock-output-names = "rtc32k";
>  	};
>
> +	thermal-zones {
> +		cpu_thermal: cpu_thermal {
> +			polling-delay-passive = <1000>; /* milliseconds */
> +			polling-delay = <1000>; /* milliseconds */
> +
> +			thermal-sensors = <&thermal 0>;
> +			sustainable-power = <1000>;
> +
> +			trips {
> +				threshold: trip-point@0 {
> +					temperature = <68000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				target: trip-point@1 {
> +					temperature = <85000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit: cpu_crit@0 {
> +					temperature = <115000>;
> +					hysteresis = <2000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +	};
> +
>  	timer {
>  		compatible = "arm,armv7-timer";
>  		interrupt-parent = <&gic>;
> @@ -270,6 +300,19 @@
>  		status = "disabled";
>  	};
>
> +	thermal: thermal@1100b000 {
> +		#thermal-sensor-cells = <0>;
> +		compatible = "mediatek,mt2701-thermal";
> +		reg = <0 0x1100b000 0 0x1000>;
> +		interrupts = <GIC_SPI 70 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&pericfg CLK_PERI_THERM>, <&pericfg CLK_PERI_AUXADC>;
> +		clock-names = "therm", "auxadc";
> +		resets = <&pericfg 0x10>;

should be MT2701_PERI_AUXADC_SW_RST, right?

> +		reset-names = "therm";
> +		mediatek,auxadc = <&auxadc>;
> +		mediatek,apmixedsys = <&apmixedsys>;
> +	};
> +
>  	nandc: nfi@1100d000 {
>  		compatible = "mediatek,mt2701-nfc";
>  		reg = <0 0x1100d000 0 0x1000>;
>

^ permalink raw reply

* Re: [PATCH v2 1/6] arm: dts: mt2701: Add spi device node
From: Matthias Brugger @ 2017-01-13 15:29 UTC (permalink / raw)
  To: Erin Lo
  Cc: srv_heupstream-NuS5LvNUpcJWk0Htik3J/w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Leilk Liu
In-Reply-To: <1484296978-18572-2-git-send-email-erin.lo-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>



On 13/01/17 09:42, Erin Lo wrote:
> From: Leilk Liu <leilk.liu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> Add spi device node for MT2701.
>
> Signed-off-by: Leilk Liu <leilk.liu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Erin Lo <erin.lo-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  arch/arm/boot/dts/mt2701-evb.dts | 50 ++++++++++++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/mt2701.dtsi    | 39 +++++++++++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
>

Applied to v4.10-next/dts32

> diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
> index 082ca88..879f1eb 100644
> --- a/arch/arm/boot/dts/mt2701-evb.dts
> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> @@ -24,6 +24,56 @@
>  	};
>  };
>
> +&pio {
> +	spi_pins_a: spi0@0 {
> +		pins_spi {
> +			pinmux = <MT2701_PIN_53_SPI0_CSN__FUNC_SPI0_CS>,
> +				 <MT2701_PIN_54_SPI0_CK__FUNC_SPI0_CK>,
> +				 <MT2701_PIN_55_SPI0_MI__FUNC_SPI0_MI>,
> +				 <MT2701_PIN_56_SPI0_MO__FUNC_SPI0_MO>;
> +			bias-disable;
> +		};
> +	};
> +
> +	spi_pins_b: spi1@0 {
> +		pins_spi {
> +			pinmux = <MT2701_PIN_7_SPI1_CSN__FUNC_SPI1_CS>,
> +				 <MT2701_PIN_8_SPI1_MI__FUNC_SPI1_MI>,
> +				 <MT2701_PIN_9_SPI1_MO__FUNC_SPI1_MO>,
> +				 <MT2701_PIN_199_SPI1_CLK__FUNC_SPI1_CK>;
> +			bias-disable;
> +		};
> +	};
> +
> +	spi_pins_c: spi2@0 {
> +		pins_spi {
> +			pinmux = <MT2701_PIN_101_SPI2_CSN__FUNC_SPI2_CS>,
> +				 <MT2701_PIN_102_SPI2_MI__FUNC_SPI2_MI>,
> +				 <MT2701_PIN_103_SPI2_MO__FUNC_SPI2_MO>,
> +				 <MT2701_PIN_104_SPI2_CLK__FUNC_SPI2_CK>;
> +			bias-disable;
> +		};
> +	};
> +};
> +
> +&spi0 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi_pins_a>;
> +	status = "disabled";
> +};
> +
> +&spi1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi_pins_b>;
> +	status = "disabled";
> +};
> +
> +&spi2 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&spi_pins_c>;
> +	status = "disabled";
> +};
> +
>  &uart0 {
>  	status = "okay";
>  };
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index bdf8954..eb4c6fd 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -227,6 +227,45 @@
>  		status = "disabled";
>  	};
>
> +	spi0: spi@1100a000 {
> +		compatible = "mediatek,mt2701-spi";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0 0x1100a000 0 0x100>;
> +		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&topckgen CLK_TOP_SYSPLL3_D2>,
> +			 <&topckgen CLK_TOP_SPI0_SEL>,
> +			 <&pericfg CLK_PERI_SPI0>;
> +		clock-names = "parent-clk", "sel-clk", "spi-clk";
> +		status = "disabled";
> +	};
> +
> +	spi1: spi@11016000 {
> +		compatible = "mediatek,mt2701-spi";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0 0x11016000 0 0x100>;
> +		interrupts = <GIC_SPI 79 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&topckgen CLK_TOP_SYSPLL3_D2>,
> +			 <&topckgen CLK_TOP_SPI1_SEL>,
> +			 <&pericfg CLK_PERI_SPI1>;
> +		clock-names = "parent-clk", "sel-clk", "spi-clk";
> +		status = "disabled";
> +	};
> +
> +	spi2: spi@11017000 {
> +		compatible = "mediatek,mt2701-spi";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		reg = <0 0x11017000 0 0x1000>;
> +		interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&topckgen CLK_TOP_SYSPLL3_D2>,
> +			 <&topckgen CLK_TOP_SPI2_SEL>,
> +			 <&pericfg CLK_PERI_SPI2>;
> +		clock-names = "parent-clk", "sel-clk", "spi-clk";
> +		status = "disabled";
> +	};
> +
>  	mmsys: syscon@14000000 {
>  		compatible = "mediatek,mt2701-mmsys", "syscon";
>  		reg = <0 0x14000000 0 0x1000>;
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 3/6] arm: dts: mt2701: Add nand device node
From: Matthias Brugger @ 2017-01-13 15:29 UTC (permalink / raw)
  To: Erin Lo
  Cc: srv_heupstream, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek, Xiaolei Li
In-Reply-To: <1484296978-18572-4-git-send-email-erin.lo@mediatek.com>



On 13/01/17 09:42, Erin Lo wrote:
> From: Xiaolei Li <xiaolei.li@mediatek.com>
>
> Add mt2701 nand device node, include nfi and bch ecc.
>
> Signed-off-by: Xiaolei Li <xiaolei.li@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
>  arch/arm/boot/dts/mt2701.dtsi | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>

Applied to v4.10-next/dts32

> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index 87be52c..1182c43 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -261,6 +261,28 @@
>  		status = "disabled";
>  	};
>
> +	nandc: nfi@1100d000 {
> +		compatible = "mediatek,mt2701-nfc";
> +		reg = <0 0x1100d000 0 0x1000>;
> +		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&pericfg CLK_PERI_NFI>,
> +			 <&pericfg CLK_PERI_NFI_PAD>;
> +		clock-names = "nfi_clk", "pad_clk";
> +		status = "disabled";
> +		ecc-engine = <&bch>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +
> +	bch: ecc@1100e000 {
> +		compatible = "mediatek,mt2701-ecc";
> +		reg = <0 0x1100e000 0 0x1000>;
> +		interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&pericfg CLK_PERI_NFI_ECC>;
> +		clock-names = "nfiecc_clk";
> +		status = "disabled";
> +	};
> +
>  	spi1: spi@11016000 {
>  		compatible = "mediatek,mt2701-spi";
>  		#address-cells = <1>;
>

^ permalink raw reply

* Re: [PATCH v2 4/6] arm: dts: mt2701: Add auxadc device node.
From: Matthias Brugger @ 2017-01-13 15:29 UTC (permalink / raw)
  To: Erin Lo
  Cc: srv_heupstream, devicetree, linux-arm-kernel, linux-kernel,
	linux-mediatek, Zhiyong Tao
In-Reply-To: <1484296978-18572-5-git-send-email-erin.lo@mediatek.com>



On 13/01/17 09:42, Erin Lo wrote:
> From: Zhiyong Tao <zhiyong.tao@mediatek.com>
>
> Add auxadc device node for MT2701.
>
> Signed-off-by: Zhiyong Tao <zhiyong.tao@mediatek.com>
> Signed-off-by: Erin Lo <erin.lo@mediatek.com>
> ---
>  arch/arm/boot/dts/mt2701-evb.dts | 4 ++++
>  arch/arm/boot/dts/mt2701.dtsi    | 9 +++++++++
>  2 files changed, 13 insertions(+)
>

Applied to v4.10-next/dts32

> diff --git a/arch/arm/boot/dts/mt2701-evb.dts b/arch/arm/boot/dts/mt2701-evb.dts
> index 879f1eb..a483798 100644
> --- a/arch/arm/boot/dts/mt2701-evb.dts
> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> @@ -24,6 +24,10 @@
>  	};
>  };
>
> +&auxadc {
> +	status = "okay";
> +};
> +
>  &pio {
>  	spi_pins_a: spi0@0 {
>  		pins_spi {
> diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
> index 1182c43..4f52019 100644
> --- a/arch/arm/boot/dts/mt2701.dtsi
> +++ b/arch/arm/boot/dts/mt2701.dtsi
> @@ -208,6 +208,15 @@
>  		      <0 0x10216000 0 0x2000>;
>  	};
>
> +	auxadc: adc@11001000 {
> +		compatible = "mediatek,mt2701-auxadc";
> +		reg = <0 0x11001000 0 0x1000>;
> +		clocks = <&pericfg CLK_PERI_AUXADC>;
> +		clock-names = "main";
> +		#io-channel-cells = <1>;
> +		status = "disabled";
> +	};
> +
>  	uart0: serial@11002000 {
>  		compatible = "mediatek,mt2701-uart",
>  			     "mediatek,mt6577-uart";
>

^ permalink raw reply

* Re: [PATCH v5] arm64: dts: mt8173: add mmsel clocks for 4K support
From: Matthias Brugger @ 2017-01-13 15:32 UTC (permalink / raw)
  To: Daniel Kurtz, Bibby Hsieh
  Cc: Mark Rutland, open list:OPEN FIRMWARE AND...,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	moderated list:ARM/Mediatek SoC support, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Catalin Marinas, Will Deacon,
	Yingjoe Chen, Sascha Hauer, James Liao, Lorenzo Pieralisi,
	YH Huang
In-Reply-To: <CAGS+omAZvHmSz6bJcvisC1LZgxwbd-HzvYMDrHXzfkY3mZZEsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>



On 12/01/17 05:50, Daniel Kurtz wrote:
> Hi Matthias,
>
> (Trying again to send plain text email)...
>
> On Thu, Aug 4, 2016 at 10:57 AM, Bibby Hsieh <bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> To support HDMI 4K resolution, mmsys need clcok
>> mm_sel to be 400MHz.
>>
>> The board .dts file should override the clock rate
>> property with the higher VENCPLL frequency the board
>> supports HDMI 4K resolution.
>>
>> Signed-off-by: Bibby Hsieh <bibby.hsieh-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>
> Reviewed-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>

Applied to v4.10-next/dts
Thanks

>
> It looks like this patch was lost.  It is actually a prerequisite for
> MTK 4k HDMI support, which already landed in v4.9.
>
> See the email thread entitled:
> [PATCH v5 0/3] MT8173 HDMI 4K support <https://lkml.org/lkml/2016/9/28/893>
>
> Or these three:
>
> 0d2200794f0a drm/mediatek: modify the factor to make the pll_rate set
> in the 1G-2G range
> 968253bd7caa drm/mediatek: enhance the HDMI driving current
> d542b7c473f0 drm/mediatek: do mtk_hdmi_send_infoframe after HDMI clock enable
>
> -Dan
>
>> ---
>>  arch/arm64/boot/dts/mediatek/mt8173.dtsi |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> index 78529e4..c3f32f3 100644
>> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
>> @@ -690,6 +690,8 @@
>>                         compatible = "mediatek,mt8173-mmsys", "syscon";
>>                         reg = <0 0x14000000 0 0x1000>;
>>                         power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>;
>> +                       assigned-clocks = <&topckgen CLK_TOP_MM_SEL>;
>> +                       assigned-clock-rates = <400000000>;
>>                         #clock-cells = <1>;
>>                 };
>>
>> --
>> 1.7.9.5
>>
--
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] arm64: dts: mt8173: Fix cpu_thermal cooling-maps contributions
From: Matthias Brugger @ 2017-01-13 15:36 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Dawei Chien, Javi Merino, Rob Herring, Mark Rutland,
	Catalin Marinas, Will Deacon, Hans Verkuil, Mauro Carvalho Chehab,
	Tiffany Lin, CK Hu, Andrew-CT Chen, Sascha Hauer, Minghsiu Tsai,
	Yong Wu, Chunfeng Yun,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)
In-Reply-To: <20170113023006.150845-1-djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>



On 13/01/17 03:30, Daniel Kurtz wrote:
> According to [0], the contribution field for each cooling-device express
> their relative power efficiency. Higher weights express higher power
> efficiency.  Weighting is relative such that if each cooling device has a
> weight of 1 they are considered equal. This is particularly useful in
> heterogeneous systems where two cooling devices may perform the same kind
> of compute, but with different efficiency.
>
> [0] Documentation/thermal/power_allocator.txt
>
> According to Mediatek IC designer, the power efficiency ratio between the
> LITTLE core cluster (cooling-device cpu0) and big core cluster
> (cooling-device cpu1) is around 3:1 (3072:1024).
>
> Signed-off-by: Daniel Kurtz <djkurtz-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

applied, thanks.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> index 12e702771f5c..9a3b0d20f7a8 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
> @@ -182,12 +182,12 @@
>  				map@0 {
>  					trip = <&target>;
>  					cooling-device = <&cpu0 0 0>;
> -					contribution = <1024>;
> +					contribution = <3072>;
>  				};
>  				map@1 {
>  					trip = <&target>;
>  					cooling-device = <&cpu2 0 0>;
> -					contribution = <2048>;
> +					contribution = <1024>;
>  				};
>  			};
>  		};
>
--
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] i2c: mux: pca954x: Add interrupt controller support
From: Phil Reid @ 2017-01-13 15:43 UTC (permalink / raw)
  To: Peter Rosin, wsa-z923LK4zBo2bacvFa/9K2g,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <383ee3cb-b452-259f-486f-682ea9526708-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 13/01/2017 17:11, Peter Rosin wrote:
> On 2017-01-09 10:02, Phil Reid wrote:
>> Various muxes can aggregate multiple interrupts from each i2c bus.
>> All of the muxes with interrupt support combine the active low irq lines
>> using an internal 'and' function and generate a combined active low
>> output. The muxes do provide the ability to read a control register to
>> determine which irq is active. By making the mux an irq controller isr
>> latency can potentially be reduced by reading the status register and
>> then only calling the registered isr on that bus segment.
>>
>> As there is no irq masking on the mux irq are disabled until irq_unmask is
>> called at least once.
>>
>
> I had a second reading of this patch. I'm still no master-of-irqs, though.
> Anyway, I have some questions below. I guess it mostly shows that I don't
> really know what I'm talking about here...
I'm no expert either...

>
>> Signed-off-by: Phil Reid <preid-qgqNFa1JUf/o2iN0hyhwsIdd74u8MsAO@public.gmane.org>
>> ---
>>  drivers/i2c/muxes/i2c-mux-pca954x.c | 127 +++++++++++++++++++++++++++++++++++-
>>  1 file changed, 125 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> index bbf088e..84fc767 100644
>> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
>> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
>> @@ -41,14 +41,19 @@
>>  #include <linux/i2c.h>
>>  #include <linux/i2c-mux.h>
>>  #include <linux/i2c/pca954x.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_irq.h>
>>  #include <linux/pm.h>
>>  #include <linux/slab.h>
>>
>>  #define PCA954X_MAX_NCHANS 8
>>
>> +#define PCA954X_IRQ_OFFSET 4
>> +
>>  enum pca_type {
>>  	pca_9540,
>>  	pca_9542,
>> @@ -63,6 +68,7 @@ enum pca_type {
>>  struct chip_desc {
>>  	u8 nchans;
>>  	u8 enable;	/* used for muxes only */
>> +	u8 has_irq;
>>  	enum muxtype {
>>  		pca954x_ismux = 0,
>>  		pca954x_isswi
>> @@ -75,6 +81,9 @@ struct pca954x {
>>  	u8 last_chan;		/* last register value */
>>  	u8 deselect;
>>  	struct i2c_client *client;
>> +
>> +	struct irq_domain *irq;
>> +	unsigned int irq_mask;
>>  };
>>
>>  /* Provide specs for the PCA954x types we know about */
>> @@ -87,19 +96,23 @@ struct pca954x {
>>  	[pca_9542] = {
>>  		.nchans = 2,
>>  		.enable = 0x4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_ismux,
>>  	},
>>  	[pca_9543] = {
>>  		.nchans = 2,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_isswi,
>>  	},
>>  	[pca_9544] = {
>>  		.nchans = 4,
>>  		.enable = 0x4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_ismux,
>>  	},
>>  	[pca_9545] = {
>>  		.nchans = 4,
>> +		.has_irq = 1,
>>  		.muxtype = pca954x_isswi,
>>  	},
>>  	[pca_9547] = {
>> @@ -222,6 +235,102 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
>>  	return pca954x_reg_write(muxc->parent, client, data->last_chan);
>>  }
>>
>> +static irqreturn_t pca954x_irq_handler(int irq, void *dev_id)
>> +{
>> +	struct pca954x *data = dev_id;
>> +	unsigned int child_irq;
>> +	int ret, i, handled;
>> +
>> +	ret = i2c_smbus_read_byte(data->client);
>> +	if (ret < 0)
>> +		return IRQ_NONE;
>> +
>> +	for (i = 0; i < data->chip->nchans; i++) {
>> +		if (ret & BIT(PCA954X_IRQ_OFFSET + i)) {
>> +			child_irq = irq_linear_revmap(data->irq, i);
>> +			handle_nested_irq(child_irq);
>> +			handled++;
>> +		}
>> +	}
>> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static void pca954x_irq_mask(struct irq_data *idata)
>> +{
>> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> +	unsigned int pos = idata->hwirq;
>> +
>> +	data->irq_mask &= ~BIT(pos);
>> +	if (!data->irq_mask)
>> +		disable_irq(data->client->irq);
>> +}
>> +
>> +static void pca954x_irq_unmask(struct irq_data *idata)
>> +{
>> +	struct pca954x *data = irq_data_get_irq_chip_data(idata);
>> +	unsigned int pos = idata->hwirq;
>> +
>> +	if (!data->irq_mask)
>> +		enable_irq(data->client->irq);
>> +	data->irq_mask |= BIT(pos);
>> +}
>
> I assume the irq core makes sure that .irq_mask and .irq_unmask may not
> be called concurrently?
I also wasn't 100% sure about this myself.
Looking again at other drivers and the core irq code I'm still not sure.
But I think on review a lock of some kind is needed.
I think a spin_lock_irqsave would do it.
Will test next week and post a new version.

>
>> +
>> +static int pca954x_irq_set_type(struct irq_data *idata, unsigned int type)
>> +{
>> +	if ((type & IRQ_TYPE_SENSE_MASK) != IRQ_TYPE_LEVEL_LOW)
>> +		return -EINVAL;
>> +	return 0;
>> +}
>> +
>> +static struct irq_chip pca954x_irq_chip = {
>> +	.name = "i2c-mux-pca954x",
>> +	.irq_mask = pca954x_irq_mask,
>> +	.irq_unmask = pca954x_irq_unmask,
>> +	.irq_set_type = pca954x_irq_set_type,
>> +};
>> +
>> +static int pca954x_irq_setup(struct i2c_mux_core *muxc)
>> +{
>> +	struct pca954x *data = i2c_mux_priv(muxc);
>> +	struct i2c_client *client = data->client;
>> +	int c, err, irq;
>> +
>> +	if (!data->chip->has_irq || client->irq <= 0)
>> +		return 0;
>
> I assume "client->irq <= 0" means that users not specifying any interrupts
> continue to behave as they use to, right?
>
> BTW, what does client->irq == 0 represent?

This one I'm fairly confident on. Initially I had "client->irq < 0"
Which caused problems with an i2c mux that doesn't have irq's defined in the dt.

This commit seems to confirm that.
commit dab472eb931b ("i2c / ACPI: Use 0 to indicate that device does not have interrupt assigned")

And various patches converting to "<= 0" checks.
eg:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/364630.html

>
>
>
>> +
>> +	data->irq = irq_domain_add_linear(client->dev.of_node,
>> +					  data->chip->nchans,
>> +					  &irq_domain_simple_ops, data);
>> +	if (!data->irq)
>> +		return -ENODEV;
>> +
>> +	for (c = 0; c < data->chip->nchans; c++) {
>> +		irq = irq_create_mapping(data->irq, c);
>> +		irq_set_chip_data(irq, data);
>> +		irq_set_chip_and_handler(irq, &pca954x_irq_chip,
>> +			handle_simple_irq);
>> +	}
>> +
>> +	err = devm_request_threaded_irq(&client->dev, data->client->irq, NULL,
>> +					pca954x_irq_handler,
>> +					IRQF_ONESHOT | IRQF_SHARED,
>> +					"pca954x", data);
>> +	if (err)
>> +		goto err_req_irq;
>> +
>> +	disable_irq(data->client->irq);
>> +
>> +	return 0;
>> +err_req_irq:
>> +	for (c = 0; c < data->chip->nchans; c++) {
>> +		irq = irq_find_mapping(data->irq, c);
>> +		irq_dispose_mapping(irq);
>> +	}
>> +	irq_domain_remove(data->irq);
>> +
>> +	return err;
>> +}
>> +
>>  /*
>>   * I2C init/probing/exit functions
>>   */
>> @@ -286,6 +395,10 @@ static int pca954x_probe(struct i2c_client *client,
>>  	idle_disconnect_dt = of_node &&
>>  		of_property_read_bool(of_node, "i2c-mux-idle-disconnect");
>>
>> +	ret = pca954x_irq_setup(muxc);
>> +	if (ret)
>> +		goto fail_del_adapters;
>> +
>>  	/* Now create an adapter for each channel */
>>  	for (num = 0; num < data->chip->nchans; num++) {
>>  		bool idle_disconnect_pd = false;
>> @@ -311,7 +424,7 @@ static int pca954x_probe(struct i2c_client *client,
>>  			dev_err(&client->dev,
>>  				"failed to register multiplexed adapter"
>>  				" %d as bus %d\n", num, force);
>> -			goto virt_reg_failed;
>> +			goto fail_del_adapters;
>>  		}
>>  	}
>>
>> @@ -322,7 +435,7 @@ static int pca954x_probe(struct i2c_client *client,
>>
>>  	return 0;
>>
>> -virt_reg_failed:
>> +fail_del_adapters:
>>  	i2c_mux_del_adapters(muxc);
>>  	return ret;
>>  }
>> @@ -330,6 +443,16 @@ static int pca954x_probe(struct i2c_client *client,
>>  static int pca954x_remove(struct i2c_client *client)
>>  {
>>  	struct i2c_mux_core *muxc = i2c_get_clientdata(client);
>> +	struct pca954x *data = i2c_mux_priv(muxc);
>> +	int c, irq;
>> +
>> +	if (data->irq) {
>> +		for (c = 0; c < data->chip->nchans; c++) {
>> +			irq = irq_find_mapping(data->irq, c);
>> +			irq_dispose_mapping(irq);
>> +		}
>> +		irq_domain_remove(data->irq);
>> +	}
>>
>>  	i2c_mux_del_adapters(muxc);
>>  	return 0;
>>
>
>
>


-- 
Regards
Phil Reid


--
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 v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-13 16:13 UTC (permalink / raw)
  To: Matthias Brugger, Boris Brezillon, Guochun Mao
  Cc: David Woodhouse, Brian Norris, Richard Weinberger,
	Cyrille Pitchen, Rob Herring, Mark Rutland, Russell King,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <d7b866ad-5b2f-62d3-3aa3-6fa42803d625-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 01/13/2017 04:12 PM, Matthias Brugger wrote:
> 
> 
> On 13/01/17 15:17, Boris Brezillon wrote:
>> On Fri, 13 Jan 2017 15:13:29 +0800
>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>
>>> Add Mediatek nor flash node.
>>>
>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  arch/arm/boot/dts/mt2701-evb.dts |   25 +++++++++++++++++++++++++
>>>  arch/arm/boot/dts/mt2701.dtsi    |   12 ++++++++++++
>>>  2 files changed, 37 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>> index 082ca88..85e5ae8 100644
>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>> @@ -24,6 +24,31 @@
>>>      };
>>>  };
>>>
>>> +&nor_flash {
>>> +    pinctrl-names = "default";
>>> +    pinctrl-0 = <&nor_pins_default>;
>>> +    status = "okay";
>>> +    flash@0 {
>>> +        compatible = "jedec,spi-nor";
>>> +        reg = <0>;
>>> +    };
>>> +};
>>> +
>>> +&pio {
>>> +    nor_pins_default: nor {
>>> +        pins1 {
>>> +            pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>> +                 <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>> +                 <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>> +                 <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>> +                 <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>> +                 <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>> +            drive-strength = <MTK_DRIVE_4mA>;
>>> +            bias-pull-up;
>>> +        };
>>> +    };
>>> +};
>>> +
>>>  &uart0 {
>>>      status = "okay";
>>>  };
>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>> b/arch/arm/boot/dts/mt2701.dtsi
>>> index bdf8954..1eefce4 100644
>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>> @@ -227,6 +227,18 @@
>>>          status = "disabled";
>>>      };
>>>
>>> +    nor_flash: spi@11014000 {
>>> +        compatible = "mediatek,mt2701-nor",
>>> +                 "mediatek,mt8173-nor";
>>
>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>> subset of the features supported by "mediatek,mt2701-nor"?
>>
> 
> I think even if the ip block is the same, we should provide both
> bindings, just in case in the future we find out that mt2701 has some
> hidden bug, feature or bug-feature. This way even if we update the
> driver, we stay compatible with older device tree blobs in the wild.
> 
> We can drop the mt2701-nor in the bindings definition if you want.

This exactly. We should have a DT compat in the form:
compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
Then if we find a problem in the future, we can match on the
"vendor,<soc>-block" and still support the old DTs.

The question is, does the "vendor,<soc>-block" go into the binding
document as well or do we only have "vendor,<oldest-compat-soc>-block"
there ?

-- 
Best regards,
Marek Vasut
--
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 0/4] Add support for es8388 on the rock2
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier

This set of patches adds a machine driver for rockchip boards that use
ES8388 codecs. It also adds slave mode to the es8328 driver that
currently only supported the master mode. Then, it adds the required DT
definitions to link rockchip-i2s to the es8388 analog output.

This work is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.co.uk> with some improvements, changes and more
commits.

Romain Perier (4):
  ASoC: es8328-i2c: Add compatible for ES8388
  ASoC: es8328: Add support for slave mode
  ASoC: rockchip: Add machine driver for ES8388 codecs
  arm: dts: Add support for ES8388 to the Radxa Rock 2

 Documentation/devicetree/bindings/sound/es8328.txt |   2 +-
 .../devicetree/bindings/sound/rockchip-es8388.txt  |  28 +++
 arch/arm/boot/dts/rk3288-rock2-square.dts          |  37 +++
 sound/soc/codecs/es8328-i2c.c                      |   3 +-
 sound/soc/codecs/es8328.c                          |  28 ++-
 sound/soc/rockchip/Kconfig                         |   9 +
 sound/soc/rockchip/Makefile                        |   2 +
 sound/soc/rockchip/rockchip_es8388.c               | 270 +++++++++++++++++++++
 8 files changed, 371 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
 create mode 100644 sound/soc/rockchip/rockchip_es8388.c

-- 
2.9.3

^ permalink raw reply

* [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>

This commit adds a compatible string for everest,es8388. This is
an audio codec that is compatible with es8328.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
 sound/soc/codecs/es8328-i2c.c                      | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/es8328.txt b/Documentation/devicetree/bindings/sound/es8328.txt
index 30ea8a3..33fbf05 100644
--- a/Documentation/devicetree/bindings/sound/es8328.txt
+++ b/Documentation/devicetree/bindings/sound/es8328.txt
@@ -4,7 +4,7 @@ This device supports both I2C and SPI.
 
 Required properties:
 
-  - compatible : "everest,es8328"
+  - compatible  : Should be "everest,es8328" or "everest,es8388"
   - DVDD-supply : Regulator providing digital core supply voltage 1.8 - 3.6V
   - AVDD-supply : Regulator providing analog supply voltage 3.3V
   - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
index 2d05b5d..09ae67f 100644
--- a/sound/soc/codecs/es8328-i2c.c
+++ b/sound/soc/codecs/es8328-i2c.c
@@ -19,13 +19,14 @@
 #include "es8328.h"
 
 static const struct i2c_device_id es8328_id[] = {
-	{ "es8328", 0 },
+	{ "es8328", "es8388", 0 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, es8328_id);
 
 static const struct of_device_id es8328_of_match[] = {
 	{ .compatible = "everest,es8328", },
+	{ .compatible = "everest,es8388", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, es8328_of_match);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/4] ASoC: es8328: Add support for slave mode
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>

Currently, the function that changes the DAI format only supports master
mode. Trying to use a slave mode exits the function with -EINVAL and
leave the codec misconfigured. This commits adds support for enabling
the slave mode.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 sound/soc/codecs/es8328.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/sound/soc/codecs/es8328.c b/sound/soc/codecs/es8328.c
index 37722194..054e123 100644
--- a/sound/soc/codecs/es8328.c
+++ b/sound/soc/codecs/es8328.c
@@ -588,10 +588,18 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	struct snd_soc_codec *codec = codec_dai->codec;
 	u8 dac_mode = 0;
 	u8 adc_mode = 0;
+	bool master;
 
-	/* set master/slave audio interface */
-	if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)
-		return -EINVAL;
+	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;
+	}
 
 	/* interface format */
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
@@ -620,9 +628,17 @@ static int es8328_set_dai_fmt(struct snd_soc_dai *codec_dai,
 	snd_soc_update_bits(codec, ES8328_ADCCONTROL4,
 			ES8328_ADCCONTROL4_ADCFORMAT_MASK, adc_mode);
 
-	/* 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);
+	}
 
 	return 0;
 }
-- 
2.9.3

^ permalink raw reply related

* [PATCH 3/4] ASoC: rockchip: Add machine driver for ES8388 codecs
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>

The driver is used for Rockchip boards using an audio codec compatible
with ES8388.

This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.co.uk> with some improvements.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 .../devicetree/bindings/sound/rockchip-es8388.txt  |  28 +++
 sound/soc/rockchip/Kconfig                         |   9 +
 sound/soc/rockchip/Makefile                        |   2 +
 sound/soc/rockchip/rockchip_es8388.c               | 270 +++++++++++++++++++++
 4 files changed, 309 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/rockchip-es8388.txt
 create mode 100644 sound/soc/rockchip/rockchip_es8388.c

diff --git a/Documentation/devicetree/bindings/sound/rockchip-es8388.txt b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
new file mode 100644
index 0000000..03a0507
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/rockchip-es8388.txt
@@ -0,0 +1,28 @@
+ROCKCHIP with ES8388 CODECS
+
+Required properties:
+- compatible: "rockchip,rockchip-audio-es8388"
+- rockchip,model: The user-visible name of this sound complex
+- rockchip,i2s-controller: The phandle of the Rockchip I2S controller that's
+  connected to the CODEC
+- rockchip,audio-codec: The phandle of the es8388 audio codec
+
+Optionnal properties:
+- rockchip,hp-en-gpios = The phandle of the GPIO that power up/down the
+  headphone.
+- rockchip,hp-det-gpios = The phandle of the GPIO that detects the headphone
+- pinctrl-names, pinctrl-0: Please refer to pinctrl-bindings.txt
+
+Example:
+	
+sound {
+	compatible = "rockchip,rockchip-audio-es8388";
+	rockchip,model = "Analog audio output";
+	rockchip,i2s-controller = <&i2s>;
+	rockchip,audio-codec = <&es8388>;
+	rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+	rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&headphone>;
+};
+
diff --git a/sound/soc/rockchip/Kconfig b/sound/soc/rockchip/Kconfig
index c783f9a..ec982c5 100644
--- a/sound/soc/rockchip/Kconfig
+++ b/sound/soc/rockchip/Kconfig
@@ -42,6 +42,15 @@ config SND_SOC_ROCKCHIP_RT5645
 	  Say Y or M here if you want to add support for SoC audio on Rockchip
 	  boards using the RT5645/RT5650 codec, such as Veyron.
 
+config SND_SOC_ROCKCHIP_ES8388
+	tristate "ASoC support for Rockchip boards using a ES8388 codec"
+	depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB
+	select SND_SOC_ROCKCHIP_I2S
+	select SND_SOC_ES8328_I2C if I2C
+	help
+	  Say Y or M here if you want to add support for SoC audio on Rockchip
+	  boards using the ES8388 Codec such as Radxa Rock 2
+
 config SND_SOC_RK3399_GRU_SOUND
 	tristate "ASoC support multiple codecs for Rockchip RK3399 GRU boards"
 	depends on SND_SOC_ROCKCHIP && I2C && GPIOLIB && CLKDEV_LOOKUP && SPI
diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index 84e5c7c..759db5e 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -7,8 +7,10 @@ obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
 
 snd-soc-rockchip-max98090-objs := rockchip_max98090.o
 snd-soc-rockchip-rt5645-objs := rockchip_rt5645.o
+snd-soc-rockchip-es8388-objs := rockchip_es8388.o
 snd-soc-rk3399-gru-sound-objs := rk3399_gru_sound.o
 
 obj-$(CONFIG_SND_SOC_ROCKCHIP_MAX98090) += snd-soc-rockchip-max98090.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_RT5645) += snd-soc-rockchip-rt5645.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_ES8388) += snd-soc-rockchip-es8388.o
 obj-$(CONFIG_SND_SOC_RK3399_GRU_SOUND) += snd-soc-rk3399-gru-sound.o
diff --git a/sound/soc/rockchip/rockchip_es8388.c b/sound/soc/rockchip/rockchip_es8388.c
new file mode 100644
index 0000000..b23ce8e
--- /dev/null
+++ b/sound/soc/rockchip/rockchip_es8388.c
@@ -0,0 +1,270 @@
+/*
+ * Rockchip machine ASoC driver for boards using a ES8388 CODEC.
+ *
+ * Copyright (c) 2016, Collabora Ltd.
+ *
+ * Authors: Sjoerd Simons <sjoerd.simons@collabora.com>,
+ *	    Romain Perier <romain.perier@collabora.com>
+ *
+ * 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/>.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+
+#include "rockchip_i2s.h"
+
+#define DRV_NAME "rockchip-snd-es8388"
+
+struct rk_es8388_drvdata {
+	int gpio_hp_en;
+	int gpio_hp_det;
+};
+
+static int rk_es8388_hp_power(struct snd_soc_dapm_widget *w,
+			      struct snd_kcontrol *k, int event)
+{
+	struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(w->dapm->card);
+
+	if (!gpio_is_valid(machine->gpio_hp_en))
+		return 0;
+
+	gpio_set_value_cansleep(machine->gpio_hp_en,
+				SND_SOC_DAPM_EVENT_ON(event));
+
+	return 0;
+}
+
+static struct snd_soc_jack headphone_jack;
+static struct snd_soc_jack_pin headphone_jack_pins[] = {
+	{
+		.pin = "Headphone",
+		.mask = SND_JACK_HEADPHONE
+	},
+};
+
+static const struct snd_soc_dapm_widget rk_dapm_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", rk_es8388_hp_power),
+};
+
+static const struct snd_soc_dapm_route rk_audio_map[] = {
+	{"Headphone", NULL, "LOUT2"},
+	{"Headphone", NULL, "ROUT2"},
+};
+
+static const struct snd_kcontrol_new rk_mc_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+};
+
+static int rk_hw_params(struct snd_pcm_substream *substream,
+			struct snd_pcm_hw_params *params)
+{
+	int ret = 0;
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+	struct snd_soc_dai *codec_dai = rtd->codec_dai;
+	int mclk;
+
+	switch (params_rate(params)) {
+	case 8000:
+	case 16000:
+	case 24000:
+	case 32000:
+	case 48000:
+	case 64000:
+	case 96000:
+		mclk = 12288000;
+		break;
+	case 11025:
+	case 22050:
+	case 44100:
+	case 88200:
+		mclk = 11289600;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk,
+				     SND_SOC_CLOCK_OUT);
+
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk,
+				     SND_SOC_CLOCK_IN);
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(codec_dai->dev, "Can't set codec clock %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_jack_gpio rk_hp_jack_gpio = {
+	.name = "Headphone detection",
+	.report = SND_JACK_HEADPHONE,
+	.debounce_time = 150
+};
+
+static int rk_init(struct snd_soc_pcm_runtime *runtime)
+{
+	struct rk_es8388_drvdata *machine = snd_soc_card_get_drvdata(runtime->card);
+
+	/* Enable Headset Jack detection */
+	if (gpio_is_valid(machine->gpio_hp_det)) {
+		snd_soc_card_jack_new(runtime->card, "Headphone Jack",
+				      SND_JACK_HEADPHONE, &headphone_jack,
+				      headphone_jack_pins,
+				      ARRAY_SIZE(headphone_jack_pins));
+		rk_hp_jack_gpio.gpio = machine->gpio_hp_det;
+		snd_soc_jack_add_gpios(&headphone_jack, 1, &rk_hp_jack_gpio);
+	}
+
+	return 0;
+}
+
+static struct snd_soc_ops rk_ops = {
+	.hw_params = rk_hw_params,
+};
+
+static struct snd_soc_dai_link rk_dailink = {
+	.name = "ES8388",
+	.stream_name = "Audio",
+	.codec_dai_name = "es8328-hifi-analog",
+	.init = rk_init,
+	.ops = &rk_ops,
+	/* Set es8388 as slave */
+	.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+		SND_SOC_DAIFMT_CBS_CFS,
+};
+
+static struct snd_soc_card snd_soc_card_rk = {
+	.name = "I2S-ES8388",
+	.dai_link = &rk_dailink,
+	.num_links = 1,
+	.num_aux_devs = 0,
+	.dapm_widgets = rk_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(rk_dapm_widgets),
+	.dapm_routes = rk_audio_map,
+	.num_dapm_routes = ARRAY_SIZE(rk_audio_map),
+	.controls = rk_mc_controls,
+	.num_controls = ARRAY_SIZE(rk_mc_controls),
+};
+
+static int snd_rk_mc_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct snd_soc_card *card = &snd_soc_card_rk;
+	struct device_node *np = pdev->dev.of_node;
+	struct rk_es8388_drvdata *machine;
+
+	machine = devm_kzalloc(&pdev->dev, sizeof(struct rk_es8388_drvdata),
+			       GFP_KERNEL);
+
+	if (!machine)
+		return -ENOMEM;
+
+	card->dev = &pdev->dev;
+
+	machine->gpio_hp_det = of_get_named_gpio(np,
+		"rockchip,hp-det-gpios", 0);
+	if (machine->gpio_hp_det == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	machine->gpio_hp_en = of_get_named_gpio(np,
+		"rockchip,hp-en-gpios", 0);
+	if (machine->gpio_hp_en == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
+	if (gpio_is_valid(machine->gpio_hp_en)) {
+		ret = devm_gpio_request_one(&pdev->dev, machine->gpio_hp_en,
+					    GPIOF_OUT_INIT_LOW, "hp_en");
+		if (ret) {
+			dev_err(card->dev, "cannot get hp_en gpio\n");
+			return ret;
+		}
+	}
+
+	ret = snd_soc_of_parse_card_name(card, "rockchip,model");
+	if (ret) {
+		dev_err(card->dev, "SoC parse card name failed %d\n", ret);
+		return ret;
+	}
+
+	rk_dailink.codec_of_node = of_parse_phandle(np, "rockchip,audio-codec",
+						    0);
+	if (!rk_dailink.codec_of_node) {
+		dev_err(&pdev->dev,
+			"Property 'rockchip,audio-codec' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	rk_dailink.cpu_of_node = of_parse_phandle(np, "rockchip,i2s-controller",
+						  0);
+	if (!rk_dailink.cpu_of_node) {
+		dev_err(&pdev->dev,
+			"Property 'rockchip,i2s-controller' missing or invalid\n");
+		return -EINVAL;
+	}
+
+	rk_dailink.platform_of_node = rk_dailink.cpu_of_node;
+	snd_soc_card_set_drvdata(card, machine);
+
+	ret = devm_snd_soc_register_card(&pdev->dev, card);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Soc register card failed %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, card);
+
+	return ret;
+}
+
+static const struct of_device_id rockchip_es8388_of_match[] = {
+	{ .compatible = "rockchip,rockchip-audio-es8388", },
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, rockchip_es8388_of_match);
+
+static struct platform_driver snd_rk_es8388_driver = {
+	.probe = snd_rk_mc_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.pm = &snd_soc_pm_ops,
+		.of_match_table = rockchip_es8388_of_match,
+	},
+};
+
+module_platform_driver(snd_rk_es8388_driver);
+
+MODULE_AUTHOR("Sjoerd Simons");
+MODULE_DESCRIPTION("Rockchip es8388 machine ASoC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.9.3

^ permalink raw reply related

* [PATCH 4/4] arm: dts: Add support for ES8388 to the Radxa Rock 2
From: Romain Perier @ 2017-01-13 16:14 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, alsa-devel, Heiko Stuebner
  Cc: Mark Rutland, devicetree, Pawel Moll, Ian Campbell,
	linux-rockchip, Sjoerd Simons, Rob Herring, Kumar Gala,
	Romain Perier
In-Reply-To: <20170113161452.14313-1-romain.perier@collabora.com>

This commit adds the DT definition of the es8388 i2c device
found at address 0x10. It also adds the definition for connecting
the Rockchip I2S to the es8388 analog output.

This commit is based on the initial work that was done by Sjoerd Simons
<sjoerd.simons@collabora.com> with some improvements.

Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
 arch/arm/boot/dts/rk3288-rock2-square.dts | 37 +++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288-rock2-square.dts b/arch/arm/boot/dts/rk3288-rock2-square.dts
index 96a2e74..4f9b7d2 100644
--- a/arch/arm/boot/dts/rk3288-rock2-square.dts
+++ b/arch/arm/boot/dts/rk3288-rock2-square.dts
@@ -86,6 +86,17 @@
 		#sound-dai-cells = <0>;
 	};
 
+	sound_es8388 {
+		compatible = "rockchip,rockchip-audio-es8388";
+		rockchip,model = "Analog audio output";
+		rockchip,i2s-controller = <&i2s>;
+		rockchip,audio-codec = <&es8388>;
+		rockchip,hp-en-gpios = <&gpio8 0 GPIO_ACTIVE_HIGH>;
+		rockchip,hp-det-gpios = <&gpio7 7 GPIO_ACTIVE_HIGH>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&headphone>;
+	};
+
 	sdio_pwrseq: sdio-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		clocks = <&hym8563>;
@@ -173,10 +184,29 @@
 	};
 };
 
+&i2c2 {
+	status = "okay";
+
+	es8388: es8388@10 {
+		compatible = "everest,es8388", "everest,es8328";
+		reg = <0x10>;
+		AVDD-supply = <&vcca_codec>;
+		DVDD-supply = <&vcca_codec>;
+		HPVDD-supply = <&vcca_codec>;
+		PVDD-supply = <&vcca_codec>;
+		clocks = <&cru SCLK_I2S0_OUT>;
+		clock-names = "i2s_clk_out";
+	};
+};
+
 &i2c5 {
 	status = "okay";
 };
 
+&i2s {
+	status = "okay";
+};
+
 &pinctrl {
 	ir {
 		ir_int: ir-int {
@@ -190,6 +220,13 @@
 		};
 	};
 
+	sound {
+		headphone: headphone {
+			rockchip,pins = <8 0 RK_FUNC_GPIO &pcfg_pull_up>,
+					<7 7 RK_FUNC_GPIO &pcfg_pull_none>;
+		};
+	};
+
 	usb {
 		host_vbus_drv: host-vbus-drv {
 			rockchip,pins = <0 14 RK_FUNC_GPIO &pcfg_pull_none>;
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH linux v1 1/2] Documentation: dt-bindings: Document bindings for ASPEED AST2400/AST2500 pwm and fan tach controller device driver
From: Rob Herring @ 2017-01-13 16:21 UTC (permalink / raw)
  To: Jaghathiswari Rankappagounder Natarajan
  Cc: openbmc, joel, jdelvare, linux, linux-hwmon, linux-kernel, corbet,
	linux-doc, mark.rutland, devicetree
In-Reply-To: <20170109215935.30067-2-jaghu@google.com>

On Mon, Jan 09, 2017 at 01:59:34PM -0800, Jaghathiswari Rankappagounder Natarajan wrote:
> This binding provides interface for adding values related to ASPEED
> AST2400/2500 PWM and Fan tach controller support.
> The PWM controller can support upto 8 PWM output ports.
> The Fan tach controller can support upto 16 tachometer inputs.
> PWM clock types M, N and 0 are three types just to have three independent
> PWM sources.
> 
> Signed-off-by: Jaghathiswari Rankappagounder Natarajan <jaghu@google.com>
> ---
>  .../devicetree/bindings/hwmon/aspeed-pwm-tacho.txt | 153 +++++++++++++++++++++

Perhaps bindings/pwm/... even though this is more than just PWM.

>  1 file changed, 153 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> new file mode 100644
> index 000000000000..8f346409ee8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed-pwm-tacho.txt
> @@ -0,0 +1,153 @@
> +ASPEED AST2400/AST2500 PWM and Fan Tacho controller device driver
> +
> +The ASPEED PWM controller can support upto 8 PWM outputs. The ASPEED Fan Tacho
> +controller can support upto 16 tachometer inputs. The PWM controller supports
> +3 types of frequency mode PWM for fan speed control. PWM clock types M, N and 0
> +are 3 types of frequency mode PWM just to have 3 independent PWM sources.
> +
> +Required properties for pwm_tacho node:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 1.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the AST2400/AST2500 PWM
> +	      ports.
> +
> +- compatible : should be "aspeed,aspeed2400-pwm-tacho" for AST2400 or
> +	       "aspeed,aspeed2500-pwm-tacho" for AST2500.
> +
> +- clocks : a fixed clock providing input clock frequency(PWM
> +	   and Fan Tach clock)
> +
> +type_values subnode format:

Don't use '_' in node or property names.

> +===========================
> +Under type_values subnode there can be upto 3 child nodes indicating type M/N/O
> +values. Atleast one child node is required.
> +
> +Required properties for the child node(type M/N/O):
> +- pwm_period : indicates type M/N/O PWM period, as per the AST2400/AST2500
> +	       datasheet. integer value in the range 0 to 255.


> +
> +- pwm_clock_division_l : indicates type M/N/O PWM clock division L value,
> +			 as per the AST2400/AST2500 datasheet.
> +			 integer value in the range 0 to 15.
> +			 0 here indicates divide 1, 1 indicates divide 2,
> +			 2 indicates divide 4, 3 indicates divide 6, and so on
> +			 till 15 indicates divide 30.
> +
> +- pwm_clock_division_h : indicates type M/N/O PWM clock division H value,
> +			 as per the AST2400/AST2500 datasheet.
> +			 integer value in the range 0 to 15.
> +			 0 here indicates divide 1, 1 indicates divide 2,
> +			 2 indicates divide 4, 3 indicates divide 8, and so on
> +			 till 15 indicates divide 32768.

Can't you have a single divider value and driver convert to register 
values? Really, you should specify the PWM period in ns/us and calculate 
the divider based on the input clock freq. (i.e. use the clock binding). 
There's already PWM binding to specify the period.

I think you should have a node for the fan using the PWM binding and 
perhaps moving some of these properties to the fan node.

> +
> +- fan_tach_enable : indicates fan tach enable of type M/N/O as per the
> +		    AST2400/AST2500 datasheet. boolean value.
> +
> +- fan_tach_clock_division : indicates fan tach clock division as per the
> +			    AST2400/AST2500 datasheet.
> +			    integer value in the range 0 to 7.
> +			    0 indicates divide 4, 1 indicates divide 16,
> +			    2 indicates divide 64, 3 indicates divide 256
> +			    and so on till 7 indicates divide 65536.
> +
> +- fan_tach_mode_selection : indicates fan tach mode mode selection as per the
> +			    AST2400/AST2500 datasheet. integer value in the
> +			    range 0 to 2. 0 indicates falling edge, 1 indicates
> +			    rising edge and 2 indicates both edges.
> +
> +- fan_tach_period : indicates fan tach period as per the AST2400/AST2500
> +		    datasheet. integer value (can be upto 16 bits long).
> +
> +pwm_port subnode format:
> +========================
> +Under pwm_port subnode there can upto 8 child nodes each indicating values
> +for one of the 8 PWM output ports.
> +
> +Required properties for each child node(starting from PWM A through PWM H):
> +- enable : enable PWM #X port, X ranges from A through H. boolean value.

A connection in the PWM binding would imply this.

> +
> +- type : indicates type selection value of PWM #X port, X ranges from A
> +	 through H. integer value in the range 0 to 2;
> +	 0 indicates type M, 1 indicates type N, 2 indicates type O.

This is meaningless to me.

> +- fan_ctrl : set the PWM fan control initial value. integer value between
> +	     0(off) and 255(full speed).
> +
> +fan_tach_channel subnode format:
> +================================
> +Under fan_tach_channel subnode there can be upto 16 child nodes each indicating
> +values for one of the 16 fan tach channels.
> +
> +Required properties for each child node(starting from fan tach #0 through
> +fan tach #16):
> +- fan-ctrl-gpios : should specify the tachometer input pin on the hardware.

I don't understand the connection here. You have a tach block with 16 
inputs on the SoC, so why the GPIOs? If the GPIOs go to the fan, then a 
fan node should have the GPIO properties.

> +
> +- enable : enable fan tach #X, X ranges from 0 through 16. boolean value.

'status' property is the standard way to enable a node or not. Not sure 
if that makes sense here though.

> +
> +- pwm_source : indicates PWM source of fan tach #X, X ranges from 0 through 16.
> +	       integer value in the range 0 to 7. 0 indicates PWM port A,
> +	       1 indicates PWM port B and so on till 7 indicates PWM port H.

^ permalink raw reply

* Re: [PATCH v1 2/2] arm: dts: mt2701: add nor flash node
From: Boris Brezillon @ 2017-01-13 16:28 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
	Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
	Russell King, linux-mtd, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel
In-Reply-To: <b850b751-1d8d-0053-cf1f-d5314542e5c3@gmail.com>

On Fri, 13 Jan 2017 17:13:55 +0100
Marek Vasut <marek.vasut@gmail.com> wrote:

> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
> > 
> > 
> > On 13/01/17 15:17, Boris Brezillon wrote:  
> >> On Fri, 13 Jan 2017 15:13:29 +0800
> >> Guochun Mao <guochun.mao@mediatek.com> wrote:
> >>  
> >>> Add Mediatek nor flash node.
> >>>
> >>> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> >>> ---
> >>>  arch/arm/boot/dts/mt2701-evb.dts |   25 +++++++++++++++++++++++++
> >>>  arch/arm/boot/dts/mt2701.dtsi    |   12 ++++++++++++
> >>>  2 files changed, 37 insertions(+)
> >>>
> >>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
> >>> b/arch/arm/boot/dts/mt2701-evb.dts
> >>> index 082ca88..85e5ae8 100644
> >>> --- a/arch/arm/boot/dts/mt2701-evb.dts
> >>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
> >>> @@ -24,6 +24,31 @@
> >>>      };
> >>>  };
> >>>
> >>> +&nor_flash {
> >>> +    pinctrl-names = "default";
> >>> +    pinctrl-0 = <&nor_pins_default>;
> >>> +    status = "okay";
> >>> +    flash@0 {
> >>> +        compatible = "jedec,spi-nor";
> >>> +        reg = <0>;
> >>> +    };
> >>> +};
> >>> +
> >>> +&pio {
> >>> +    nor_pins_default: nor {
> >>> +        pins1 {
> >>> +            pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
> >>> +                 <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
> >>> +                 <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
> >>> +                 <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
> >>> +                 <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
> >>> +                 <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
> >>> +            drive-strength = <MTK_DRIVE_4mA>;
> >>> +            bias-pull-up;
> >>> +        };
> >>> +    };
> >>> +};
> >>> +
> >>>  &uart0 {
> >>>      status = "okay";
> >>>  };
> >>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
> >>> b/arch/arm/boot/dts/mt2701.dtsi
> >>> index bdf8954..1eefce4 100644
> >>> --- a/arch/arm/boot/dts/mt2701.dtsi
> >>> +++ b/arch/arm/boot/dts/mt2701.dtsi
> >>> @@ -227,6 +227,18 @@
> >>>          status = "disabled";
> >>>      };
> >>>
> >>> +    nor_flash: spi@11014000 {
> >>> +        compatible = "mediatek,mt2701-nor",
> >>> +                 "mediatek,mt8173-nor";  
> >>
> >> Why define both here? Is "mediatek,mt8173-nor" really providing a
> >> subset of the features supported by "mediatek,mt2701-nor"?
> >>  
> > 
> > I think even if the ip block is the same, we should provide both
> > bindings, just in case in the future we find out that mt2701 has some
> > hidden bug, feature or bug-feature. This way even if we update the
> > driver, we stay compatible with older device tree blobs in the wild.
> > 
> > We can drop the mt2701-nor in the bindings definition if you want. 

Oh, sorry, I misunderstood. What I meant is that if you want to
list/support all possible compatibles, maybe you should just put one
compatible in your DT and patch your driver (+ binding doc) to define
all of them.

> 
> This exactly. We should have a DT compat in the form:
> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
> Then if we find a problem in the future, we can match on the
> "vendor,<soc>-block" and still support the old DTs.

Not sure it's only in term of whose IP appeared first. My understanding
is that it's a way to provide inheritance. For example:

	"<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";

or

	"<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";

BTW, which one is the oldest between mt8173 and mt2701? :-)

^ permalink raw reply

* Re: [PATCH 02/37] PCI: dwc: designware: Add new *ops* for cpu addr fixup
From: Joao Pinto @ 2017-01-13 16:34 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
	Arnd Bergmann
  Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
	linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
	linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-3-git-send-email-kishon@ti.com>


Hi Kishon,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Some platforms (like dra7xx) require only the least 28 bits of the
> corresponding 32 bit CPU address to be programmed in the address
> translation unit. This modified address is stored in io_base/mem_base/
> cfg0_base/cfg1_base in dra7xx_pcie_host_init. While this is okay for
> host mode where the address range is fixed, device mode requires
> different addresses to be programmed based on the host buffer address.
> Add a new ops to get the least 28 bits of the corresponding 32 bit
> CPU address and invoke it before programming the address translation
> unit.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/dwc/pcie-designware.c |    3 +++
>  drivers/pci/dwc/pcie-designware.h |    1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index bed1999..d68bc7b 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -195,6 +195,9 @@ static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
>  {
>  	u32 retries, val;
>  
> +	if (pp->ops->cpu_addr_fixup)
> +		cpu_addr = pp->ops->cpu_addr_fixup(cpu_addr);
> +
>  	if (pp->iatu_unroll_enabled) {
>  		dw_pcie_writel_unroll(pp, index, PCIE_ATU_UNR_LOWER_BASE,
>  			lower_32_bits(cpu_addr));
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index a567ea2..32f4602 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -54,6 +54,7 @@ struct pcie_port {
>  };
>  
>  struct pcie_host_ops {
> +	u64 (*cpu_addr_fixup)(u64 cpu_addr);
>  	u32 (*readl_rc)(struct pcie_port *pp, u32 reg);
>  	void (*writel_rc)(struct pcie_port *pp, u32 reg, u32 val);
>  	int (*rd_own_conf)(struct pcie_port *pp, int where, int size, u32 *val);
> 

I think this is an acceptable fixup, I am ok with it.

Reviewed-By: Joao Pinto <jpinto@synopsys.com>

Joao

^ permalink raw reply

* Re: [PATCH 04/37] PCI: dwc: designware: Move the register defines to designware header file
From: Joao Pinto @ 2017-01-13 16:35 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
	Arnd Bergmann
  Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
	linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
	linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-5-git-send-email-kishon@ti.com>


Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Move the register defines and other macros from
> pcie-designware.c to pcie-designware.h. This is in preparation to
> split the pcie-designware.c file into designware core file and host
> specific file.
> 
> While at that also fix a checkpatch warning.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/dwc/pcie-designware.c |   70 ------------------------------------
>  drivers/pci/dwc/pcie-designware.h |   71 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 71 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index d68bc7b..0b928dc 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -25,76 +25,6 @@
>  
>  #include "pcie-designware.h"

Make sense.

Reviewed-By: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH 06/37] PCI: dwc: Rename cfg_read/cfg_write to read/write
From: Joao Pinto @ 2017-01-13 16:36 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
	Arnd Bergmann
  Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
	linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
	linux-arm-kernel, linux-arm-msm, nsekhar, Murali Karicheri,
	Stanimir Varbanov, Pratyush Anand
In-Reply-To: <1484216786-17292-7-git-send-email-kishon@ti.com>


Hi,

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. dw_pcie_cfg_read/dw_pcie_cfg_write doesn't do
> anything specific to access configuration space. It can be just renamed
> to dw_pcie_read/dw_pcie_write and used to read/write data to dbi space.
> This is in preparation for added endpoint support to linux kernel.
> 
> Cc: Jingoo Han <jingoohan1@gmail.com>
> Cc: Murali Karicheri <m-karicheri2@ti.com>
> Cc: Joao Pinto <Joao.Pinto@synopsys.com>
> Cc: Stanimir Varbanov <svarbanov@mm-sol.com>
> Cc: Pratyush Anand <pratyush.anand@gmail.com>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/dwc/pci-dra7xx.c      |   16 ++++++++--------
>  drivers/pci/dwc/pci-exynos.c      |    4 ++--
>  drivers/pci/dwc/pci-keystone-dw.c |    4 ++--
>  drivers/pci/dwc/pcie-designware.c |   12 ++++++------
>  drivers/pci/dwc/pcie-designware.h |    4 ++--
>  drivers/pci/dwc/pcie-qcom.c       |    2 +-
>  drivers/pci/dwc/pcie-spear13xx.c  |   24 ++++++++++++------------
>  7 files changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index aeeab74..38b0c9a 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -114,22 +114,22 @@ static int dra7xx_pcie_establish_link(struct dra7xx_pcie *dra7xx)
>  	}
>  
>  	if (dra7xx->link_gen == 1) {
> -		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> -				 4, &reg);
> +		dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> +			     4, &reg);
>  		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>  			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
>  			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> -			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -					  PCI_EXP_LNKCAP, 4, reg);
> +			dw_pcie_write(pp->dbi_base + exp_cap_off +
> +				      PCI_EXP_LNKCAP, 4, reg);
>  		}
>  
> -		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> -				 2, &reg);
> +		dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> +			     2, &reg);
>  		if ((reg & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>  			reg &= ~((u32)PCI_EXP_LNKCAP_SLS);
>  			reg |= PCI_EXP_LNKCAP_SLS_2_5GB;
> -			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -					  PCI_EXP_LNKCTL2, 2, reg);
> +			dw_pcie_write(pp->dbi_base + exp_cap_off +
> +				      PCI_EXP_LNKCTL2, 2, reg);
>  		}
>  	}
>  
> diff --git a/drivers/pci/dwc/pci-exynos.c b/drivers/pci/dwc/pci-exynos.c
> index c179e7a..e3fbff4 100644
> --- a/drivers/pci/dwc/pci-exynos.c
> +++ b/drivers/pci/dwc/pci-exynos.c
> @@ -429,7 +429,7 @@ static int exynos_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>  	int ret;
>  
>  	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, true);
> -	ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> +	ret = dw_pcie_read(pp->dbi_base + where, size, val);
>  	exynos_pcie_sideband_dbi_r_mode(exynos_pcie, false);
>  	return ret;
>  }
> @@ -441,7 +441,7 @@ static int exynos_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  	int ret;
>  
>  	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, true);
> -	ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> +	ret = dw_pcie_write(pp->dbi_base + where, size, val);
>  	exynos_pcie_sideband_dbi_w_mode(exynos_pcie, false);
>  	return ret;
>  }
> diff --git a/drivers/pci/dwc/pci-keystone-dw.c b/drivers/pci/dwc/pci-keystone-dw.c
> index 9397c46..4875334 100644
> --- a/drivers/pci/dwc/pci-keystone-dw.c
> +++ b/drivers/pci/dwc/pci-keystone-dw.c
> @@ -444,7 +444,7 @@ int ks_dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  
>  	addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>  
> -	return dw_pcie_cfg_read(addr + where, size, val);
> +	return dw_pcie_read(addr + where, size, val);
>  }
>  
>  int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> @@ -456,7 +456,7 @@ int ks_dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  
>  	addr = ks_pcie_cfg_setup(ks_pcie, bus_num, devfn);
>  
> -	return dw_pcie_cfg_write(addr + where, size, val);
> +	return dw_pcie_write(addr + where, size, val);
>  }
>  
>  /**
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index 0b928dc..d0ea310 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -33,7 +33,7 @@
>  
>  static struct pci_ops dw_pcie_ops;
>  
> -int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
> +int dw_pcie_read(void __iomem *addr, int size, u32 *val)
>  {
>  	if ((uintptr_t)addr & (size - 1)) {
>  		*val = 0;
> @@ -54,7 +54,7 @@ int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
>  	return PCIBIOS_SUCCESSFUL;
>  }
>  
> -int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val)
> +int dw_pcie_write(void __iomem *addr, int size, u32 val)
>  {
>  	if ((uintptr_t)addr & (size - 1))
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
> @@ -108,7 +108,7 @@ static int dw_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>  	if (pp->ops->rd_own_conf)
>  		return pp->ops->rd_own_conf(pp, where, size, val);
>  
> -	return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> +	return dw_pcie_read(pp->dbi_base + where, size, val);
>  }
>  
>  static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
> @@ -117,7 +117,7 @@ static int dw_pcie_wr_own_conf(struct pcie_port *pp, int where, int size,
>  	if (pp->ops->wr_own_conf)
>  		return pp->ops->wr_own_conf(pp, where, size, val);
>  
> -	return dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> +	return dw_pcie_write(pp->dbi_base + where, size, val);
>  }
>  
>  static void dw_pcie_prog_outbound_atu(struct pcie_port *pp, int index,
> @@ -638,7 +638,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
>  				  type, cpu_addr,
>  				  busdev, cfg_size);
> -	ret = dw_pcie_cfg_read(va_cfg_base + where, size, val);
> +	ret = dw_pcie_read(va_cfg_base + where, size, val);
>  	if (pp->num_viewport <= 2)
>  		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
>  					  PCIE_ATU_TYPE_IO, pp->io_base,
> @@ -676,7 +676,7 @@ static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  	dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
>  				  type, cpu_addr,
>  				  busdev, cfg_size);
> -	ret = dw_pcie_cfg_write(va_cfg_base + where, size, val);
> +	ret = dw_pcie_write(va_cfg_base + where, size, val);
>  	if (pp->num_viewport <= 2)
>  		dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1,
>  					  PCIE_ATU_TYPE_IO, pp->io_base,
> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index a6cf9262..b6ddb05 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -146,8 +146,8 @@ struct pcie_host_ops {
>  
>  u32 dw_pcie_readl_rc(struct pcie_port *pp, u32 reg);
>  void dw_pcie_writel_rc(struct pcie_port *pp, u32 reg, u32 val);
> -int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val);
> -int dw_pcie_cfg_write(void __iomem *addr, int size, u32 val);
> +int dw_pcie_read(void __iomem *addr, int size, u32 *val);
> +int dw_pcie_write(void __iomem *addr, int size, u32 val);
>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
>  void dw_pcie_msi_init(struct pcie_port *pp);
>  int dw_pcie_wait_for_link(struct pcie_port *pp);
> diff --git a/drivers/pci/dwc/pcie-qcom.c b/drivers/pci/dwc/pcie-qcom.c
> index 2c24c45..d75fc02 100644
> --- a/drivers/pci/dwc/pcie-qcom.c
> +++ b/drivers/pci/dwc/pcie-qcom.c
> @@ -630,7 +630,7 @@ static int qcom_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>  		return PCIBIOS_SUCCESSFUL;
>  	}
>  
> -	return dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> +	return dw_pcie_read(pp->dbi_base + where, size, val);
>  }
>  
>  static struct pcie_host_ops qcom_pcie_dw_ops = {
> diff --git a/drivers/pci/dwc/pcie-spear13xx.c b/drivers/pci/dwc/pcie-spear13xx.c
> index 5970566..7acf91e 100644
> --- a/drivers/pci/dwc/pcie-spear13xx.c
> +++ b/drivers/pci/dwc/pcie-spear13xx.c
> @@ -91,34 +91,34 @@ static int spear13xx_pcie_establish_link(struct spear13xx_pcie *spear13xx_pcie)
>  	 * default value in capability register is 512 bytes. So force
>  	 * it to 128 here.
>  	 */
> -	dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
> +	dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, &val);
>  	val &= ~PCI_EXP_DEVCTL_READRQ;
> -	dw_pcie_cfg_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
> +	dw_pcie_write(pp->dbi_base + exp_cap_off + PCI_EXP_DEVCTL, 2, val);
>  
> -	dw_pcie_cfg_write(pp->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
> -	dw_pcie_cfg_write(pp->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
> +	dw_pcie_write(pp->dbi_base + PCI_VENDOR_ID, 2, 0x104A);
> +	dw_pcie_write(pp->dbi_base + PCI_DEVICE_ID, 2, 0xCD80);
>  
>  	/*
>  	 * if is_gen1 is set then handle it, so that some buggy card
>  	 * also works
>  	 */
>  	if (spear13xx_pcie->is_gen1) {
> -		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> -					4, &val);
> +		dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCAP,
> +			     4, &val);
>  		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>  			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
>  			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> -			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -				PCI_EXP_LNKCAP, 4, val);
> +			dw_pcie_write(pp->dbi_base + exp_cap_off +
> +				      PCI_EXP_LNKCAP, 4, val);
>  		}
>  
> -		dw_pcie_cfg_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> -					2, &val);
> +		dw_pcie_read(pp->dbi_base + exp_cap_off + PCI_EXP_LNKCTL2,
> +			     2, &val);
>  		if ((val & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB) {
>  			val &= ~((u32)PCI_EXP_LNKCAP_SLS);
>  			val |= PCI_EXP_LNKCAP_SLS_2_5GB;
> -			dw_pcie_cfg_write(pp->dbi_base + exp_cap_off +
> -					PCI_EXP_LNKCTL2, 2, val);
> +			dw_pcie_write(pp->dbi_base + exp_cap_off +
> +				      PCI_EXP_LNKCTL2, 2, val);
>  		}
>  	}
>  
> 

Make sense.

Reviewed-By: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

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

On Tue, Jan 10, 2017 at 07:44:01AM -0800, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [170109 15:43]:
> > 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.
> > 
> > For hardware reference, see omap4430 TRM "Table 3-1312. L4PER_CM2 Registers
> > Mapping Summary" for example. It show one instance of a clkctrl clock
> > controller with multiple clkctrl registers.
> > 
> > Note that this binding allows keeping the clockdomain related parts out of
> > drivers/clock. The CLKCTCTRL and DYNAMICDEP registers can be handled by
> > using 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.
> > 
> > On omap4 CM_L3INIT_USB_HOST_HS_CLKCTRL on omap5 has eight OPTFCLKEN bits.
> > So we need to shift the clock index to avoid index conflict for the clock
> > consumer binding with the next clkctrl offset on omap4.
> > 
> > Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > ---
> > 
> > So here's what I was able to come up for the clkctr binding based on
> > all we've discussed so far. Can you guys please take a look and see
> > if it looks OK to you before we do the device driver?
> > 
> > Also, does anybody have better suggestions for addressing the optional
> > clocks in each clkctrl register?
> 
> The other option that might be worth considering is to make use of the
> #clock-cells property. Then the index of any optional clock could be passed
> in the second cell.
> 
> The third cell could be used to set the modulemode for the clock (software
> controlled or hardware controlled) instead of using a custom property
> at the clock controllel level.

I guess I prefer this way. Or you could do a mixture of both proposals 
with 2 cells. The first being the clock id and the 2nd flags. 

What's the max optional clocks in theory? B picked from the current 
worst case seems a bit worrying. Why not 16? Upper half is offset, lower 
half is index. 

> In that case clock consume usage would look like the following using
> #clock-cells = <3>:
> 
> #define OMAP4_CLKCTRL_OFFSET		0x20
> #define MODULEMODE_HWCTRL		1
> #define MODULEMODE_SWCTRL		2

Can you make one of these 0 instead or is both being set valid?

> 
> #define OMAP_CLKCTRL_INDEX(offset)	((offset) - OMAP4_CLKCTRL_OFFSET)
> 
> #define OMAP4_GPTIMER10_CLKTRL		OMAP_CLKCTRL_INDEX(0x28)
> #define OMAP4_GPTIMER11_CLKTRL		OMAP_CLKCTRL_INDEX(0x30)
> #define OMAP4_GPTIMER2_CLKTRL		OMAP_CLKCTRL_INDEX(0x38)
> ...
> #define OMAP4_GPIO2_CLKCTRL		OMAP_CLKCTRL_INDEX(0x60)
> ...
> 
> &gpio2 {
> 	clocks = <&cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL 0 MODULEMODE_HWCTRL
> 		  &cm_l4per_clkctrl OMAP4_GPIO2_CLKCTRL_DBCLK 1 0>;

Drop the _DBCLK here, right?

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

^ permalink raw reply

* Re: [PATCH 10/37] PCI: dwc: designware: Fix style errors in pcie-designware.c
From: Joao Pinto @ 2017-01-13 16:38 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
	Arnd Bergmann
  Cc: linux-pci, linux-doc, linux-kernel, devicetree, linux-omap,
	linux-arm-kernel, linux-samsung-soc, linuxppc-dev,
	linux-arm-kernel, linux-arm-msm, nsekhar
In-Reply-To: <1484216786-17292-11-git-send-email-kishon@ti.com>

Às 10:25 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> No functional change. Fix all checkpatch warnings and check errors
> in pcie-designware.c
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/dwc/pcie-designware.c |   42 ++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pcie-designware.c b/drivers/pci/dwc/pcie-designware.c
> index 89cdb6b..ff04074 100644
> --- a/drivers/pci/dwc/pcie-designware.c
> +++ b/drivers/pci/dwc/pcie-designware.c
> @@ -40,13 +40,13 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
>  	}
>  
> -	if (size == 4)
> +	if (size == 4) {
>  		*val = readl(addr);
> -	else if (size == 2)
> +	} else if (size == 2) {
>  		*val = readw(addr);
> -	else if (size == 1)
> +	} else if (size == 1) {
>  		*val = readb(addr);
> -	else {
> +	} else {
>  		*val = 0;
>  		return PCIBIOS_BAD_REGISTER_NUMBER;
>  	}
> @@ -203,16 +203,15 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  
>  	for (i = 0; i < MAX_MSI_CTRLS; i++) {
>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
> -				(u32 *)&val);
> +				    (u32 *)&val);
>  		if (val) {
>  			ret = IRQ_HANDLED;
>  			pos = 0;
>  			while ((pos = find_next_bit(&val, 32, pos)) != 32) {
>  				irq = irq_find_mapping(pp->irq_domain,
> -						i * 32 + pos);
> -				dw_pcie_wr_own_conf(pp,
> -						PCIE_MSI_INTR0_STATUS + i * 12,
> -						4, 1 << pos);
> +						       i * 32 + pos);
> +				dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS +
> +						    i * 12, 4, 1 << pos);
>  				generic_handle_irq(irq);
>  				pos++;
>  			}
> @@ -278,8 +277,9 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
>  static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
>  {
>  	int irq, pos0, i;
> -	struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(desc);
> +	struct pcie_port *pp;
>  
> +	pp  = (struct pcie_port *)msi_desc_to_pci_sysdata(desc);
>  	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
>  				       order_base_2(no_irqs));
>  	if (pos0 < 0)
> @@ -341,7 +341,7 @@ static void dw_msi_setup_msg(struct pcie_port *pp, unsigned int irq, u32 pos)
>  }
>  
>  static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
> -			struct msi_desc *desc)
> +			    struct msi_desc *desc)
>  {
>  	int irq, pos;
>  	struct pcie_port *pp = pdev->bus->sysdata;
> @@ -389,7 +389,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
>  {
>  	struct irq_data *data = irq_get_irq_data(irq);
>  	struct msi_desc *msi = irq_data_get_msi_desc(data);
> -	struct pcie_port *pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
> +	struct pcie_port *pp = (struct pcie_port *)msi_desc_to_pci_sysdata(msi);
>  
>  	clear_irq_range(pp, irq, 1, data->hwirq);
>  }
> @@ -431,7 +431,7 @@ int dw_pcie_link_up(struct dw_pcie *pci)
>  }
>  
>  static int dw_pcie_msi_map(struct irq_domain *domain, unsigned int irq,
> -			irq_hw_number_t hwirq)
> +			   irq_hw_number_t hwirq)
>  {
>  	irq_set_chip_and_handler(irq, &dw_msi_irq_chip, handle_simple_irq);
>  	irq_set_chip_data(irq, domain->host_data);
> @@ -468,8 +468,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  
>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>  	if (cfg_res) {
> -		pp->cfg0_size = resource_size(cfg_res)/2;
> -		pp->cfg1_size = resource_size(cfg_res)/2;
> +		pp->cfg0_size = resource_size(cfg_res) / 2;
> +		pp->cfg1_size = resource_size(cfg_res) / 2;
>  		pp->cfg0_base = cfg_res->start;
>  		pp->cfg1_base = cfg_res->start + pp->cfg0_size;
>  	} else if (!pp->va_cfg0_base) {
> @@ -508,8 +508,8 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  			break;
>  		case 0:
>  			pp->cfg = win->res;
> -			pp->cfg0_size = resource_size(pp->cfg)/2;
> -			pp->cfg1_size = resource_size(pp->cfg)/2;
> +			pp->cfg0_size = resource_size(pp->cfg) / 2;
> +			pp->cfg1_size = resource_size(pp->cfg) / 2;
>  			pp->cfg0_base = pp->cfg->start;
>  			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
>  			break;
> @@ -615,7 +615,7 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  }
>  
>  static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> -		u32 devfn, int where, int size, u32 *val)
> +				 u32 devfn, int where, int size, u32 *val)
>  {
>  	int ret, type;
>  	u32 busdev, cfg_size;
> @@ -654,7 +654,7 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
>  }
>  
>  static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> -		u32 devfn, int where, int size, u32 val)
> +				 u32 devfn, int where, int size, u32 val)
>  {
>  	int ret, type;
>  	u32 busdev, cfg_size;
> @@ -711,7 +711,7 @@ static int dw_pcie_valid_device(struct pcie_port *pp, struct pci_bus *bus,
>  }
>  
>  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> -			int size, u32 *val)
> +			   int size, u32 *val)
>  {
>  	struct pcie_port *pp = bus->sysdata;
>  
> @@ -727,7 +727,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>  }
>  
>  static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> -			int where, int size, u32 val)
> +			   int where, int size, u32 val)
>  {
>  	struct pcie_port *pp = bus->sysdata;
>  
> 

Always good to make clean up! Thanks!

Acked-By: Joao Pinto <jpinto@synopsys.com>

^ permalink raw reply

* Re: [PATCH v4 2/2] dt-bindings: clk: add rockchip,grf property for RK3399
From: Rob Herring @ 2017-01-13 16:41 UTC (permalink / raw)
  To: Xing Zheng
  Cc: heiko-4mtYJXux2i+zQB+pC5nmwQ, dianders-hpIqsD4AKlfQT0dZR+AlfA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Michael Turquette, Stephen Boyd, Mark Rutland,
	linux-clk-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1484028930-20305-3-git-send-email-zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

On Tue, Jan 10, 2017 at 02:15:30PM +0800, Xing Zheng wrote:
> Add support for rockchip,grf property which is used for GRF muxes
> on RK3399.
> 
> Signed-off-by: Xing Zheng <zhengxing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
> 
> Changes in v4:
> - update the decription for rockchip,grf property
> 
> Changes in v3: None
> Changes in v2: None
> 
>  Documentation/devicetree/bindings/clock/rockchip,rk3399-cru.txt | 6 ++++++
>  1 file changed, 6 insertions(+)

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/4] ASoC: es8328-i2c: Add compatible for ES8388
From: Heiko Stuebner @ 2017-01-13 16:43 UTC (permalink / raw)
  To: Romain Perier
  Cc: Jaroslav Kysela, Takashi Iwai, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Ian Campbell,
	Pawel Moll, Mark Rutland, Kumar Gala,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sjoerd Simons
In-Reply-To: <20170113161452.14313-2-romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Hi Romain,

Am Freitag, 13. Januar 2017, 17:14:49 CET schrieb Romain Perier:
> This commit adds a compatible string for everest,es8388. This is
> an audio codec that is compatible with es8328.
> 
> Signed-off-by: Romain Perier <romain.perier-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/sound/es8328.txt | 2 +-
>  sound/soc/codecs/es8328-i2c.c                      | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/es8328.txt
> b/Documentation/devicetree/bindings/sound/es8328.txt index 30ea8a3..33fbf05
> 100644
> --- a/Documentation/devicetree/bindings/sound/es8328.txt
> +++ b/Documentation/devicetree/bindings/sound/es8328.txt
> @@ -4,7 +4,7 @@ This device supports both I2C and SPI.
> 
>  Required properties:
> 
> -  - compatible : "everest,es8328"
> +  - compatible  : Should be "everest,es8328" or "everest,es8388"
>    - DVDD-supply : Regulator providing digital core supply voltage 1.8 -
> 3.6V - AVDD-supply : Regulator providing analog supply voltage 3.3V
>    - PVDD-supply : Regulator providing digital IO supply voltage 1.8 - 3.6V
> diff --git a/sound/soc/codecs/es8328-i2c.c b/sound/soc/codecs/es8328-i2c.c
> index 2d05b5d..09ae67f 100644
> --- a/sound/soc/codecs/es8328-i2c.c
> +++ b/sound/soc/codecs/es8328-i2c.c
> @@ -19,13 +19,14 @@
>  #include "es8328.h"
> 
>  static const struct i2c_device_id es8328_id[] = {
> -	{ "es8328", 0 },
> +	{ "es8328", "es8388", 0 },

that looks strange or I'm just blind.

struct i2c_device_id {
        char name[I2C_NAME_SIZE];
        kernel_ulong_t driver_data;     /* Data private to the driver */
};

so shouldn't this be

 static const struct i2c_device_id es8328_id[] = {
 	{ "es8328", 0 },
+	{ "es8388", 0 },

instead?

>  	{ }
>  };
>  MODULE_DEVICE_TABLE(i2c, es8328_id);
> 
>  static const struct of_device_id es8328_of_match[] = {
>  	{ .compatible = "everest,es8328", },
> +	{ .compatible = "everest,es8388", },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, es8328_of_match);


--
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 v1 2/2] arm: dts: mt2701: add nor flash node
From: Marek Vasut @ 2017-01-13 16:44 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Matthias Brugger, Guochun Mao, David Woodhouse, Brian Norris,
	Richard Weinberger, Cyrille Pitchen, Rob Herring, Mark Rutland,
	Russell King, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170113172825.75d545a3@bbrezillon>

On 01/13/2017 05:28 PM, Boris Brezillon wrote:
> On Fri, 13 Jan 2017 17:13:55 +0100
> Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>> On 01/13/2017 04:12 PM, Matthias Brugger wrote:
>>>
>>>
>>> On 13/01/17 15:17, Boris Brezillon wrote:  
>>>> On Fri, 13 Jan 2017 15:13:29 +0800
>>>> Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>>>>  
>>>>> Add Mediatek nor flash node.
>>>>>
>>>>> Signed-off-by: Guochun Mao <guochun.mao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>>>> ---
>>>>>  arch/arm/boot/dts/mt2701-evb.dts |   25 +++++++++++++++++++++++++
>>>>>  arch/arm/boot/dts/mt2701.dtsi    |   12 ++++++++++++
>>>>>  2 files changed, 37 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/mt2701-evb.dts
>>>>> b/arch/arm/boot/dts/mt2701-evb.dts
>>>>> index 082ca88..85e5ae8 100644
>>>>> --- a/arch/arm/boot/dts/mt2701-evb.dts
>>>>> +++ b/arch/arm/boot/dts/mt2701-evb.dts
>>>>> @@ -24,6 +24,31 @@
>>>>>      };
>>>>>  };
>>>>>
>>>>> +&nor_flash {
>>>>> +    pinctrl-names = "default";
>>>>> +    pinctrl-0 = <&nor_pins_default>;
>>>>> +    status = "okay";
>>>>> +    flash@0 {
>>>>> +        compatible = "jedec,spi-nor";
>>>>> +        reg = <0>;
>>>>> +    };
>>>>> +};
>>>>> +
>>>>> +&pio {
>>>>> +    nor_pins_default: nor {
>>>>> +        pins1 {
>>>>> +            pinmux = <MT2701_PIN_240_EXT_XCS__FUNC_EXT_XCS>,
>>>>> +                 <MT2701_PIN_241_EXT_SCK__FUNC_EXT_SCK>,
>>>>> +                 <MT2701_PIN_239_EXT_SDIO0__FUNC_EXT_SDIO0>,
>>>>> +                 <MT2701_PIN_238_EXT_SDIO1__FUNC_EXT_SDIO1>,
>>>>> +                 <MT2701_PIN_237_EXT_SDIO2__FUNC_EXT_SDIO2>,
>>>>> +                 <MT2701_PIN_236_EXT_SDIO3__FUNC_EXT_SDIO3>;
>>>>> +            drive-strength = <MTK_DRIVE_4mA>;
>>>>> +            bias-pull-up;
>>>>> +        };
>>>>> +    };
>>>>> +};
>>>>> +
>>>>>  &uart0 {
>>>>>      status = "okay";
>>>>>  };
>>>>> diff --git a/arch/arm/boot/dts/mt2701.dtsi
>>>>> b/arch/arm/boot/dts/mt2701.dtsi
>>>>> index bdf8954..1eefce4 100644
>>>>> --- a/arch/arm/boot/dts/mt2701.dtsi
>>>>> +++ b/arch/arm/boot/dts/mt2701.dtsi
>>>>> @@ -227,6 +227,18 @@
>>>>>          status = "disabled";
>>>>>      };
>>>>>
>>>>> +    nor_flash: spi@11014000 {
>>>>> +        compatible = "mediatek,mt2701-nor",
>>>>> +                 "mediatek,mt8173-nor";  
>>>>
>>>> Why define both here? Is "mediatek,mt8173-nor" really providing a
>>>> subset of the features supported by "mediatek,mt2701-nor"?
>>>>  
>>>
>>> I think even if the ip block is the same, we should provide both
>>> bindings, just in case in the future we find out that mt2701 has some
>>> hidden bug, feature or bug-feature. This way even if we update the
>>> driver, we stay compatible with older device tree blobs in the wild.
>>>
>>> We can drop the mt2701-nor in the bindings definition if you want. 
> 
> Oh, sorry, I misunderstood. What I meant is that if you want to
> list/support all possible compatibles, maybe you should just put one
> compatible in your DT and patch your driver (+ binding doc) to define
> all of them.

Uh, what ? I lost you here :-)

>> This exactly. We should have a DT compat in the form:
>> compatible = "vendor,<soc>-block", "vendor,<oldest-compat-soc>-block";
>> Then if we find a problem in the future, we can match on the
>> "vendor,<soc>-block" and still support the old DTs.
> 
> Not sure it's only in term of whose IP appeared first. My understanding
> is that it's a way to provide inheritance. For example:
> 
> 	"<soc-vendor>,<ip-version>", "<ip-vendor>,<ip-version>";
> 
> or
> 
> 	"<soc-vendor>,<full-featured-ip-version>","<soc-vendor>,<basic-feature-ip-version>";
> 
> BTW, which one is the oldest between mt8173 and mt2701? :-)

And that's another thing and I agree with you, but I don't think that's
what we're discussing in this thread. But (!), OT, I think we should
codify the rules in Documentation/ . This discussion came up multiple
times recently.

And my question still stands, what do we put into the DT here, IMO
compatible = "mediatek,mt2701-nor", "mediatek,mt8173-nor";
and what goes into the binding document ? I guess both too ?
-- 
Best regards,
Marek Vasut
--
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 11/37] PCI: dwc: Split pcie-designware.c into host and core files
From: Joao Pinto @ 2017-01-13 16:49 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Jingoo Han, Joao Pinto,
	Arnd Bergmann
  Cc: devicetree, linux-samsung-soc, linux-doc, linux-pci, nsekhar,
	linux-kernel, linux-arm-kernel, linux-arm-msm, linux-omap,
	linuxppc-dev, linux-arm-kernel
In-Reply-To: <1484216786-17292-12-git-send-email-kishon@ti.com>

Às 10:26 AM de 1/12/2017, Kishon Vijay Abraham I escreveu:
> Split pcie-designware.c into pcie-designware-host.c that contains
> the host specific parts of the driver and pcie-designware.c that
> contains the parts used by both host driver and endpoint driver.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/dwc/Makefile               |    2 +-
>  drivers/pci/dwc/pcie-designware-host.c |  619 ++++++++++++++++++++++++++++++++
>  drivers/pci/dwc/pcie-designware.c      |  613 +------------------------------
>  drivers/pci/dwc/pcie-designware.h      |    8 +
>  4 files changed, 634 insertions(+), 608 deletions(-)
>  create mode 100644 drivers/pci/dwc/pcie-designware-host.c
> 
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index 7d27c14..3b57e55 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -1,4 +1,4 @@

(snip...)

> -static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> -				      int type, u64 cpu_addr, u64 pci_addr,
> -				      u32 size)
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
> +			       u64 cpu_addr, u64 pci_addr, u32 size)
>  {
>  	u32 retries, val;
>  
> @@ -186,220 +151,6 @@ static void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  	dev_err(pci->dev, "iATU is not being enabled\n");
>  }

Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.

>  
> -static struct irq_chip dw_msi_irq_chip = {
> -	.name = "PCI-MSI",
> -	.irq_enable = pci_msi_unmask_irq,
> -	.irq_disable = pci_msi_mask_irq,
> -	.irq_mask = pci_msi_mask_irq,
> -	.irq_unmask = pci_msi_unmask_irq,
> -};
> -

(snip...)

> -
> -static const struct irq_domain_ops msi_domain_ops = {
> -	.map = dw_pcie_msi_map,
> -};
> -
>  static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
>  {
>  	u32 val;
> @@ -454,303 +192,11 @@ static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
>  	return 0;
>  }

Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host.

(snip...)

> diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
> index 491fbe3..808d17b 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -14,6 +14,10 @@
>  #ifndef _PCIE_DESIGNWARE_H
>  #define _PCIE_DESIGNWARE_H
>  
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <linux/pci.h>
> +
>  /* Parameters for the waiting for link up routine */
>  #define LINK_WAIT_MAX_RETRIES		10
>  #define LINK_WAIT_USLEEP_MIN		90000
> @@ -167,4 +171,8 @@ struct dw_pcie {
>  void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val);
>  int dw_pcie_link_up(struct dw_pcie *pci);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
> +			       int type, u64 cpu_addr, u64 pci_addr,
> +			       u32 size);
> +void dw_pcie_setup(struct dw_pcie *pci);


Kishon, iATU only makes sense in The Root Complex (host), so it should be inside
the pcie-designware-host as static.

>  #endif /* _PCIE_DESIGNWARE_H */
> 

Thanks,
Joao

^ 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