Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] ARM: dts: rockchip: Add missing unit address to memory node on rk3288-veyron
From: Heiko Stuebner @ 2019-07-27 15:33 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
In-Reply-To: <20190727142736.23188-2-krzk@kernel.org>

Hi Krzysztof,

Am Samstag, 27. Juli 2019, 16:27:36 CEST schrieb Krzysztof Kozlowski:
> Fix DTC warning:
> 
>     arch/arm/boot/dts/rk3288-veyron.dtsi:21.9-24.4:
>     Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name

please see the comment directly above the memory node on why that needs
to stay that way. So no, we'll keep the veyron memory node as is.


Heiko

> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
> index 8fc8eac699bf..02243ff46a65 100644
> --- a/arch/arm/boot/dts/rk3288-veyron.dtsi
> +++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
> @@ -18,7 +18,7 @@
>  	 * The default coreboot on veyron devices ignores memory@0 nodes
>  	 * and would instead create another memory node.
>  	 */
> -	memory {
> +	memory@0 {
>  		device_type = "memory";
>  		reg = <0x0 0x0 0x0 0x80000000>;
>  	};
> 

^ permalink raw reply

* Re: Re: [PATCH 5/6] pwm: sun4i: Add support to output source clock directly
From: Chen-Yu Tsai @ 2019-07-27 14:54 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: Maxime Ripard, Thierry Reding, Rob Herring, Mark Rutland,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, devicetree, linux-arm-kernel,
	linux-kernel, linux-sunxi
In-Reply-To: <4063694.66Ui2fGJfo@jernej-laptop>

On Sat, Jul 27, 2019 at 10:28 PM Jernej Škrabec <jernej.skrabec@siol.net> wrote:
>
> Dne sobota, 27. julij 2019 ob 12:50:08 CEST je Maxime Ripard napisal(a):
> > On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > > PWM core has an option to bypass whole logic and output unchanged source
> > > clock as PWM output. This is achieved by enabling bypass bit.
> > >
> > > Note that when bypass is enabled, no other setting has any meaning, not
> > > even enable bit.
> > >
> > > This mode of operation is needed to achieve high enough frequency to
> > > serve as clock source for AC200 chip, which is integrated into same
> > > package as H6 SoC.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> >
> > It doesn't seem to be available on the A10 (at least) though. The A13
> > seem to have it, so you should probably check that, and make that
> > conditional to the compatible if not available on all of them.
>
> Ok, can you suggest the name for the quirk? "has_bypass" is suspiciously
> similar to "has_prescaler_bypass".

has_direct_mod_clk_output?

> Also, how to name these sun4i_pwm_data structures? Now that there are (will
> be) three new quirks, name of the structure would be just too long, like
> "sun50i_pwm_dual_prescaler_bypass_clk_rst_bypass".

Just use the SoC model. Any later ones that have the same quirks will likely
use the same compatible string anyway.

ChenYu

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/CAGb2v65a9jF3QsEQgTim_XxXjhd9K0KwDRxtqYMqsiy2yWLeBg%40mail.gmail.com.

^ permalink raw reply

* Re: [PATCH] ARM: dts: imx: Cleanup style around assignment operator
From: Fabio Estevam @ 2019-07-27 14:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-kernel
In-Reply-To: <20190727142640.23014-1-krzk@kernel.org>

Hi Krzysztof,

On Sat, Jul 27, 2019 at 11:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> Use a space before and after assignment operator to have consistent
> style.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Thanks for doing this cleanup:

Reviewed-by: Fabio Estevam <festevam@gmail.com>

^ permalink raw reply

* Re: [PATCH 5/6] pwm: sun4i: Add support to output source clock directly
From: Jernej Škrabec @ 2019-07-27 14:28 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, wens-jdAy2FN1RRM,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20190727105008.he35sixfvoyl2lm7-YififvaboMKzQB+pC5nmwQ@public.gmane.org>

Dne sobota, 27. julij 2019 ob 12:50:08 CEST je Maxime Ripard napisal(a):
> On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> > PWM core has an option to bypass whole logic and output unchanged source
> > clock as PWM output. This is achieved by enabling bypass bit.
> > 
> > Note that when bypass is enabled, no other setting has any meaning, not
> > even enable bit.
> > 
> > This mode of operation is needed to achieve high enough frequency to
> > serve as clock source for AC200 chip, which is integrated into same
> > package as H6 SoC.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> 
> It doesn't seem to be available on the A10 (at least) though. The A13
> seem to have it, so you should probably check that, and make that
> conditional to the compatible if not available on all of them.

Ok, can you suggest the name for the quirk? "has_bypass" is suspiciously 
similar to "has_prescaler_bypass".

Also, how to name these sun4i_pwm_data structures? Now that there are (will 
be) three new quirks, name of the structure would be just too long, like 
"sun50i_pwm_dual_prescaler_bypass_clk_rst_bypass". 

Best regards,
Jernej

> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply

* [PATCH 2/2] ARM: dts: rockchip: Add missing unit address to memory node on rk3288-veyron
From: Krzysztof Kozlowski @ 2019-07-27 14:27 UTC (permalink / raw)
  To: Heiko Stuebner, linux-arm-kernel, linux-rockchip, devicetree,
	linux-kernel
  Cc: Krzysztof Kozlowski
In-Reply-To: <20190727142736.23188-1-krzk@kernel.org>

Fix DTC warning:

    arch/arm/boot/dts/rk3288-veyron.dtsi:21.9-24.4:
    Warning (unit_address_vs_reg): /memory: node has a reg or ranges property, but no unit name

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/rk3288-veyron.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 8fc8eac699bf..02243ff46a65 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -18,7 +18,7 @@
 	 * The default coreboot on veyron devices ignores memory@0 nodes
 	 * and would instead create another memory node.
 	 */
-	memory {
+	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
-- 
2.17.1

^ permalink raw reply related

* [PATCH 1/2] ARM: dts: rockchip: Cleanup style around assignment operator
From: Krzysztof Kozlowski @ 2019-07-27 14:27 UTC (permalink / raw)
  To: Heiko Stuebner, linux-arm-kernel, linux-rockchip, devicetree,
	linux-kernel
  Cc: Krzysztof Kozlowski

Use a space before and after assignment operator to have consistent
style.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/rk3036.dtsi                   |  2 +-
 arch/arm/boot/dts/rk3288-evb.dtsi               |  2 +-
 arch/arm/boot/dts/rk3288-tinker.dtsi            | 12 ++++++------
 arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index 0290ea4edd32..c776321b2cc4 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -484,7 +484,7 @@
 		compatible = "rockchip,rockchip-spi";
 		reg = <0x20074000 0x1000>;
 		interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
-		clocks =<&cru PCLK_SPI>, <&cru SCLK_SPI>;
+		clocks = <&cru PCLK_SPI>, <&cru SCLK_SPI>;
 		clock-names = "apb-pclk","spi_pclk";
 		dmas = <&pdma 8>, <&pdma 9>;
 		dma-names = "tx", "rx";
diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi
index 820440715302..2afd686b2033 100644
--- a/arch/arm/boot/dts/rk3288-evb.dtsi
+++ b/arch/arm/boot/dts/rk3288-evb.dtsi
@@ -97,7 +97,7 @@
 	};
 
 	panel: panel {
-		compatible ="lg,lp079qx1-sp0v", "simple-panel";
+		compatible = "lg,lp079qx1-sp0v", "simple-panel";
 		backlight = <&backlight>;
 		enable-gpios = <&gpio7 RK_PA4 GPIO_ACTIVE_HIGH>;
 		pinctrl-0 = <&lcd_cs>;
diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi
index 293576869546..81e4e953d4a4 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -47,13 +47,13 @@
 		compatible = "gpio-leds";
 
 		act-led {
-			gpios=<&gpio1 RK_PD0 GPIO_ACTIVE_HIGH>;
-			linux,default-trigger="mmc0";
+			gpios = <&gpio1 RK_PD0 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "mmc0";
 		};
 
 		heartbeat-led {
-			gpios=<&gpio1 RK_PD1 GPIO_ACTIVE_HIGH>;
-			linux,default-trigger="heartbeat";
+			gpios = <&gpio1 RK_PD1 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "heartbeat";
 		};
 
 		pwr-led {
@@ -443,7 +443,7 @@
 
 &saradc {
 	vref-supply = <&vcc18_ldo1>;
-	status ="okay";
+	status = "okay";
 };
 
 &sdmmc {
@@ -516,7 +516,7 @@
 };
 
 &usb_otg {
-	status= "okay";
+	status = "okay";
 };
 
 &vopb {
diff --git a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
index 1cadb522fd0d..e0183655e92c 100644
--- a/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron-chromebook.dtsi
@@ -86,7 +86,7 @@
 	};
 
 	panel: panel {
-		compatible ="innolux,n116bge", "simple-panel";
+		compatible = "innolux,n116bge", "simple-panel";
 		status = "okay";
 		power-supply = <&vcc33_lcd>;
 		backlight = <&backlight>;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 3/6] pwm: sun4i: Add a quirk for bus clock
From: Chen-Yu Tsai @ 2019-07-27 14:27 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Jernej Skrabec, Thierry Reding, Rob Herring, Mark Rutland,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, devicetree, linux-arm-kernel,
	linux-kernel, linux-sunxi
In-Reply-To: <20190727104628.jsdvpxvcpzru75v5-YififvaboMKzQB+pC5nmwQ@public.gmane.org>

On Sat, Jul 27, 2019 at 6:46 PM Maxime Ripard <mripard-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
> Hi,
>
> On Fri, Jul 26, 2019 at 08:40:42PM +0200, Jernej Skrabec wrote:
> > H6 PWM core needs bus clock to be enabled in order to work.
> >
> > Add a quirk for it.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> >  drivers/pwm/pwm-sun4i.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 1b7be8fbde86..7d3ac3f2dc3f 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -72,6 +72,7 @@ static const u32 prescaler_table[] = {
> >  };
> >
> >  struct sun4i_pwm_data {
> > +     bool has_bus_clock;
> >       bool has_prescaler_bypass;
> >       bool has_reset;
> >       unsigned int npwm;
> > @@ -79,6 +80,7 @@ struct sun4i_pwm_data {
> >
> >  struct sun4i_pwm_chip {
> >       struct pwm_chip chip;
> > +     struct clk *bus_clk;
> >       struct clk *clk;
> >       struct reset_control *rst;
> >       void __iomem *base;
> > @@ -382,6 +384,16 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
> >               reset_control_deassert(pwm->rst);
> >       }
> >
> > +     if (pwm->data->has_bus_clock) {
> > +             pwm->bus_clk = devm_clk_get(&pdev->dev, "bus");
> > +             if (IS_ERR(pwm->bus_clk)) {
> > +                     ret = PTR_ERR(pwm->bus_clk);
> > +                     goto err_bus;
> > +             }
> > +
> > +             clk_prepare_enable(pwm->bus_clk);
> > +     }
> > +
>
> The patch itself looks fine, but you should clarify which clock is
> being used by the old driver.
>
> My guess is that the "new" clock is actually the mod one, while the
> old one was both the clock of the register interface (bus) and the
> clock of the PWM generation logic (mod).

The H6 datasheet explicitly states:

    The clock source of PWM is OSC24M. The PWM is on APB1 Bus. Ensure
    that open APB1 Bus gating and de-assert reset signal when accessed
    to the PWM.

Older datasheets do not have anything about bus gating or resets. However
with slightly newer ones that have a system bus tree diagram, we can see
that PWM is on APB1 (or APB0/APBS for R_PWM). We can assume there is no
bus gate and thus it is directly attached to APB1, and that we never
modeled this part.

So the new clock is definitely the bus gate. You might want to introduce
a patch renaming sun4i_pwm_data.clk to sun4i_pwm_data.mod_clk before this
one.

ChenYu

^ permalink raw reply

* [PATCH] ARM: dts: imx: Cleanup style around assignment operator
From: Krzysztof Kozlowski @ 2019-07-27 14:26 UTC (permalink / raw)
  To: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, devicetree, linux-arm-kernel, linux-kernel
  Cc: Krzysztof Kozlowski

Use a space before and after assignment operator to have consistent
style.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/imx6sll.dtsi              |  4 ++--
 arch/arm/boot/dts/imx6sx.dtsi               |  4 ++--
 arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi |  2 +-
 arch/arm/boot/dts/imx6ul.dtsi               | 12 ++++++------
 arch/arm/boot/dts/imx7d.dtsi                |  4 ++--
 arch/arm/boot/dts/imx7s.dtsi                |  6 +++---
 arch/arm/boot/dts/imx7ulp.dtsi              |  8 ++++----
 7 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
index b0a77ff70b67..0a103a19dc0a 100644
--- a/arch/arm/boot/dts/imx6sll.dtsi
+++ b/arch/arm/boot/dts/imx6sll.dtsi
@@ -234,7 +234,7 @@
 					compatible = "fsl,imx6sl-uart", "fsl,imx6q-uart",
 						     "fsl,imx21-uart";
 					reg = <0x02018000 0x4000>;
-					interrupts =<GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
+					interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>;
 					dmas = <&sdma 31 4 0>, <&sdma 32 4 0>;
 					dma-names = "rx", "tx";
 					clocks = <&clks IMX6SLL_CLK_UART4_IPG>,
@@ -801,7 +801,7 @@
 				compatible = "fsl,imx6sll-uart", "fsl,imx6q-uart",
 					     "fsl,imx21-uart";
 				reg = <0x021f4000 0x4000>;
-				interrupts =<GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+				interrupts = <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
 				dmas = <&sdma 33 4 0>, <&sdma 34 4 0>;
 				dma-names = "rx", "tx";
 				clocks = <&clks IMX6SLL_CLK_UART5_IPG>,
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index bb25add90f19..b36f31b633d3 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -926,8 +926,8 @@
 					 <&clks IMX6SX_CLK_ENET_PTP>;
 				clock-names = "ipg", "ahb", "ptp",
 					      "enet_clk_ref", "enet_out";
-				fsl,num-tx-queues=<3>;
-				fsl,num-rx-queues=<3>;
+				fsl,num-tx-queues = <3>;
+				fsl,num-rx-queues = <3>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi b/arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi
index fc2997449b49..a2fec095e2ab 100644
--- a/arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi
+++ b/arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi
@@ -70,7 +70,7 @@
 
 &i2c1 {
 	pinctrl-names = "default";
-	pinctrl-0 =<&pinctrl_i2c1>;
+	pinctrl-0 = <&pinctrl_i2c1>;
 	clock-frequency = <100000>;
 	status = "okay";
 
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 81d4b4925127..ef6437198db1 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -510,8 +510,8 @@
 					 <&clks IMX6UL_CLK_ENET2_REF_125M>;
 				clock-names = "ipg", "ahb", "ptp",
 					      "enet_clk_ref", "enet_out";
-				fsl,num-tx-queues=<1>;
-				fsl,num-rx-queues=<1>;
+				fsl,num-tx-queues = <1>;
+				fsl,num-rx-queues = <1>;
 				status = "disabled";
 			};
 
@@ -845,8 +845,8 @@
 					 <&clks IMX6UL_CLK_ENET_REF>;
 				clock-names = "ipg", "ahb", "ptp",
 					      "enet_clk_ref", "enet_out";
-				fsl,num-tx-queues=<1>;
-				fsl,num-rx-queues=<1>;
+				fsl,num-tx-queues = <1>;
+				fsl,num-rx-queues = <1>;
 				status = "disabled";
 			};
 
@@ -858,7 +858,7 @@
 					 <&clks IMX6UL_CLK_USDHC1>,
 					 <&clks IMX6UL_CLK_USDHC1>;
 				clock-names = "ipg", "ahb", "per";
-				fsl,tuning-step= <2>;
+				fsl,tuning-step = <2>;
 				fsl,tuning-start-tap = <20>;
 				bus-width = <4>;
 				status = "disabled";
@@ -873,7 +873,7 @@
 					 <&clks IMX6UL_CLK_USDHC2>;
 				clock-names = "ipg", "ahb", "per";
 				bus-width = <4>;
-				fsl,tuning-step= <2>;
+				fsl,tuning-step = <2>;
 				fsl,tuning-start-tap = <20>;
 				status = "disabled";
 			};
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 42528d2812a2..9c8dd32cc035 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -147,8 +147,8 @@
 			<&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
 		clock-names = "ipg", "ahb", "ptp",
 			"enet_clk_ref", "enet_out";
-		fsl,num-tx-queues=<3>;
-		fsl,num-rx-queues=<3>;
+		fsl,num-tx-queues = <3>;
+		fsl,num-rx-queues = <3>;
 		status = "disabled";
 	};
 
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index c1a4fff5ceda..710f850e785c 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -151,7 +151,7 @@
 		compatible = "fsl,imx7d-tempmon";
 		interrupt-parent = <&gpc>;
 		interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
-		fsl,tempmon =<&anatop>;
+		fsl,tempmon = <&anatop>;
 		nvmem-cells = <&tempmon_calib>,
 			<&tempmon_temp_grade>;
 		nvmem-cell-names = "calib", "temp_grade";
@@ -1184,8 +1184,8 @@
 					<&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
 				clock-names = "ipg", "ahb", "ptp",
 					"enet_clk_ref", "enet_out";
-				fsl,num-tx-queues=<3>;
-				fsl,num-rx-queues=<3>;
+				fsl,num-tx-queues = <3>;
+				fsl,num-rx-queues = <3>;
 				status = "disabled";
 			};
 		};
diff --git a/arch/arm/boot/dts/imx7ulp.dtsi b/arch/arm/boot/dts/imx7ulp.dtsi
index 992747a57442..ddab7c42b955 100644
--- a/arch/arm/boot/dts/imx7ulp.dtsi
+++ b/arch/arm/boot/dts/imx7ulp.dtsi
@@ -201,12 +201,12 @@
 			clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
 				 <&scg1 IMX7ULP_CLK_NIC1_DIV>,
 				 <&pcc2 IMX7ULP_CLK_USDHC0>;
-			clock-names ="ipg", "ahb", "per";
+			clock-names = "ipg", "ahb", "per";
 			assigned-clocks = <&pcc2 IMX7ULP_CLK_USDHC0>;
 			assigned-clock-parents = <&scg1 IMX7ULP_CLK_NIC1_DIV>;
 			bus-width = <4>;
 			fsl,tuning-start-tap = <20>;
-			fsl,tuning-step= <2>;
+			fsl,tuning-step = <2>;
 			status = "disabled";
 		};
 
@@ -217,12 +217,12 @@
 			clocks = <&scg1 IMX7ULP_CLK_NIC1_BUS_DIV>,
 				 <&scg1 IMX7ULP_CLK_NIC1_DIV>,
 				 <&pcc2 IMX7ULP_CLK_USDHC1>;
-			clock-names ="ipg", "ahb", "per";
+			clock-names = "ipg", "ahb", "per";
 			assigned-clocks = <&pcc2 IMX7ULP_CLK_USDHC1>;
 			assigned-clock-parents = <&scg1 IMX7ULP_CLK_NIC1_DIV>;
 			bus-width = <4>;
 			fsl,tuning-start-tap = <20>;
-			fsl,tuning-step= <2>;
+			fsl,tuning-step = <2>;
 			status = "disabled";
 		};
 
-- 
2.17.1

^ permalink raw reply related

* [PATCH] ARM: dts: exynos: Use space after '=' in exynos4412-itop-scp-core
From: Krzysztof Kozlowski @ 2019-07-27 14:16 UTC (permalink / raw)
  To: Kukjin Kim, Krzysztof Kozlowski, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel

Replace tab with space after assignment operator.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 .../arm/boot/dts/exynos4412-itop-scp-core.dtsi | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi b/arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi
index 462a5409b1de..dfceb155b3a7 100644
--- a/arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi
+++ b/arch/arm/boot/dts/exynos4412-itop-scp-core.dtsi
@@ -377,7 +377,7 @@
 			buck1_reg: BUCK1 {
 				regulator-name = "vdd_mif";
 				regulator-min-microvolt = <850000>;
-				regulator-max-microvolt	= <1100000>;
+				regulator-max-microvolt = <1100000>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -386,7 +386,7 @@
 			buck2_reg: BUCK2 {
 				regulator-name = "vdd_arm";
 				regulator-min-microvolt = <850000>;
-				regulator-max-microvolt	= <1456250>;
+				regulator-max-microvolt = <1456250>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -395,7 +395,7 @@
 			buck3_reg: BUCK3 {
 				regulator-name = "vdd_int";
 				regulator-min-microvolt = <875000>;
-				regulator-max-microvolt	= <1200000>;
+				regulator-max-microvolt = <1200000>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -404,7 +404,7 @@
 			buck4_reg: BUCK4 {
 				regulator-name = "vdd_g3d";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <1500000>;
+				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -413,7 +413,7 @@
 			buck5_reg: BUCK5 {
 				regulator-name = "vdd_m12";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <1500000>;
+				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -422,7 +422,7 @@
 			buck6_reg: BUCK6 {
 				regulator-name = "vdd12_5m";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <1500000>;
+				regulator-max-microvolt = <1500000>;
 				regulator-always-on;
 				regulator-boot-on;
 				op_mode = <1>; /* Normal Mode */
@@ -431,7 +431,7 @@
 			buck7_reg: BUCK7 {
 				regulator-name = "pvdd_buck7";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <2000000>;
+				regulator-max-microvolt = <2000000>;
 				regulator-boot-on;
 				regulator-always-on;
 				op_mode = <1>; /* Normal Mode */
@@ -440,7 +440,7 @@
 			buck8_reg: BUCK8 {
 				regulator-name = "pvdd_buck8";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <1500000>;
+				regulator-max-microvolt = <1500000>;
 				regulator-boot-on;
 				regulator-always-on;
 				op_mode = <1>; /* Normal Mode */
@@ -449,7 +449,7 @@
 			buck9_reg: BUCK9 {
 				regulator-name = "vddf28_emmc";
 				regulator-min-microvolt = <750000>;
-				regulator-max-microvolt	= <3000000>;
+				regulator-max-microvolt = <3000000>;
 				op_mode = <1>; /* Normal Mode */
 			};
 		};
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 3/6] pwm: sun4i: Add a quirk for bus clock
From: Jernej Škrabec @ 2019-07-27 14:15 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, wens-jdAy2FN1RRM,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20190727104628.jsdvpxvcpzru75v5-YififvaboMKzQB+pC5nmwQ@public.gmane.org>

Dne sobota, 27. julij 2019 ob 12:46:28 CEST je Maxime Ripard napisal(a):
> Hi,
> 
> On Fri, Jul 26, 2019 at 08:40:42PM +0200, Jernej Skrabec wrote:
> > H6 PWM core needs bus clock to be enabled in order to work.
> > 
> > Add a quirk for it.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec-gGgVlfcn5nU@public.gmane.org>
> > ---
> > 
> >  drivers/pwm/pwm-sun4i.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > index 1b7be8fbde86..7d3ac3f2dc3f 100644
> > --- a/drivers/pwm/pwm-sun4i.c
> > +++ b/drivers/pwm/pwm-sun4i.c
> > @@ -72,6 +72,7 @@ static const u32 prescaler_table[] = {
> > 
> >  };
> >  
> >  struct sun4i_pwm_data {
> > 
> > +	bool has_bus_clock;
> > 
> >  	bool has_prescaler_bypass;
> >  	bool has_reset;
> >  	unsigned int npwm;
> > 
> > @@ -79,6 +80,7 @@ struct sun4i_pwm_data {
> > 
> >  struct sun4i_pwm_chip {
> >  
> >  	struct pwm_chip chip;
> > 
> > +	struct clk *bus_clk;
> > 
> >  	struct clk *clk;
> >  	struct reset_control *rst;
> >  	void __iomem *base;
> > 
> > @@ -382,6 +384,16 @@ static int sun4i_pwm_probe(struct platform_device
> > *pdev)> 
> >  		reset_control_deassert(pwm->rst);
> >  	
> >  	}
> > 
> > +	if (pwm->data->has_bus_clock) {
> > +		pwm->bus_clk = devm_clk_get(&pdev->dev, "bus");
> > +		if (IS_ERR(pwm->bus_clk)) {
> > +			ret = PTR_ERR(pwm->bus_clk);
> > +			goto err_bus;
> > +		}
> > +
> > +		clk_prepare_enable(pwm->bus_clk);
> > +	}
> > +
> 
> The patch itself looks fine, but you should clarify which clock is
> being used by the old driver.
> 
> My guess is that the "new" clock is actually the mod one, while the
> old one was both the clock of the register interface (bus) and the
> clock of the PWM generation logic (mod).

Well, I checked few datasheets and nowhere is explicitly stated what is the 
bus clock, but I would make same guess as you.

Anyway, since you requested that order of the clocks has to be changed, I have 
to separately obtain clocks if there is bus clock present too or not. If it 
is, both clocks have to be obtained by name, and if not, old code without name 
can be used.

Best regards,
Jernej

> 
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

^ permalink raw reply

* Re: [PATCH v3 2/2] iio: light: noa1305: Add support for NOA1305
From: Jonathan Cameron @ 2019-07-27 13:43 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Mark Rutland, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, linux-iio, linux-kernel, kernel,
	devicetree, Sergei M
In-Reply-To: <20190726205513.31291-2-martyn.welch@collabora.com>

On Fri, 26 Jul 2019 21:55:13 +0100
Martyn Welch <martyn.welch@collabora.com> wrote:

> This driver adds the initial support for the ON Semiconductor
> NOA1305 Ambient Light Sensor.
> 
> Originally written by Sergei Miroshnichenko. Found here:
>   https://github.com/EmcraftSystems/linux-upstream/commit/196d6cf897e632d2cb82d45484bd7a1bfdd5b6d9
> 
> Signed-off-by: Sergei M <fizik1@yandex.com>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
Hi.

A few bits I missed before or follow up on your changes (which were mostly
great).

Thanks,

Jonathan

> ---
> 
> Changes:
> v2:
>  - Correcting authorship and SOB.
> v3:
>  - Improve register define naming.
>  - Follow IIO convention of interleaving register bit definitions with
>    register defintions.
>  - Use proper endian swapping.
>  - Process raw sensor count into Lux.
>  - Avoid setting variables to zero when not needed.
>  - Check return value of i2c writes.
>  - Implement disabling of regulator as a devm action.
>  - Remove excessive white spacing.
> 
>  drivers/iio/light/Kconfig   |  10 ++
>  drivers/iio/light/Makefile  |   1 +
>  drivers/iio/light/noa1305.c | 278 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 289 insertions(+)
>  create mode 100644 drivers/iio/light/noa1305.c
> 
> diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig
> index 954c958cfc43..d1db0ec0d0f5 100644
> --- a/drivers/iio/light/Kconfig
> +++ b/drivers/iio/light/Kconfig
> @@ -309,6 +309,16 @@ config MAX44009
>  	 To compile this driver as a module, choose M here:
>  	 the module will be called max44009.
>  
> +config NOA1305
> +	tristate "ON Semiconductor NOA1305 ambient light sensor"
> +	depends on I2C
> +	help
> +	 Say Y here if you want to build support for the ON Semiconductor
> +	 NOA1305 ambient light sensor.
> +
> +	 To compile this driver as a module, choose M here:
> +	 The module will be called noa1305.
> +
>  config OPT3001
>  	tristate "Texas Instruments OPT3001 Light Sensor"
>  	depends on I2C
> diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile
> index e40794fbb435..00d1f9b98f39 100644
> --- a/drivers/iio/light/Makefile
> +++ b/drivers/iio/light/Makefile
> @@ -29,6 +29,7 @@ obj-$(CONFIG_LTR501)		+= ltr501.o
>  obj-$(CONFIG_LV0104CS)		+= lv0104cs.o
>  obj-$(CONFIG_MAX44000)		+= max44000.o
>  obj-$(CONFIG_MAX44009)		+= max44009.o
> +obj-$(CONFIG_NOA1305)		+= noa1305.o
>  obj-$(CONFIG_OPT3001)		+= opt3001.o
>  obj-$(CONFIG_PA12203001)	+= pa12203001.o
>  obj-$(CONFIG_RPR0521)		+= rpr0521.o
> diff --git a/drivers/iio/light/noa1305.c b/drivers/iio/light/noa1305.c
> new file mode 100644
> index 000000000000..02b0cf48c2be
> --- /dev/null
> +++ b/drivers/iio/light/noa1305.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Support for ON Semiconductor NOA1305 ambient light sensor
> + *
> + * Copyright (C) 2016 Emcraft Systems
> + * Copyright (C) 2019 Collabora Ltd.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define NOA1305_REG_POWER_CONTROL	0x0
> +#define   NOA1305_POWER_CONTROL_DOWN	0x00
> +#define   NOA1305_POWER_CONTROL_ON	0x08
> +#define NOA1305_REG_RESET		0x1
> +#define   NOA1305_RESET_RESET		0x10
> +#define NOA1305_REG_INTEGRATION_TIME	0x2
> +#define   NOA1305_INTEGR_TIME_800MS	0x00
> +#define   NOA1305_INTEGR_TIME_400MS	0x01
> +#define   NOA1305_INTEGR_TIME_200MS	0x02
> +#define   NOA1305_INTEGR_TIME_100MS	0x03
> +#define   NOA1305_INTEGR_TIME_50MS	0x04
> +#define   NOA1305_INTEGR_TIME_25MS	0x05
> +#define   NOA1305_INTEGR_TIME_12_5MS	0x06
> +#define   NOA1305_INTEGR_TIME_6_25MS	0x07
> +#define NOA1305_REG_INT_SELECT		0x3
> +#define   NOA1305_INT_SEL_ACTIVE_HIGH	0x01
> +#define   NOA1305_INT_SEL_ACTIVE_LOW	0x02
> +#define   NOA1305_INT_SEL_INACTIVE	0x03
> +#define NOA1305_REG_INT_THRESH_LSB	0x4
> +#define NOA1305_REG_INT_THRESH_MSB	0x5
> +#define NOA1305_REG_ALS_DATA_LSB	0x6
> +#define NOA1305_REG_ALS_DATA_MSB	0x7
> +#define NOA1305_REG_DEVICE_ID_LSB	0x8
> +#define NOA1305_REG_DEVICE_ID_MSB	0x9
> +
> +#define NOA1305_DEVICE_ID	0x0519
> +#define NOA1305_DRIVER_NAME	"noa1305"
> +
> +struct noa1305_priv {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +	struct regulator *vin_reg;
> +};
> +
> +static int noa1305_measure(struct noa1305_priv *priv)
> +{
> +	__le16 data;
> +	int count;
> +	int ret;
> +
> +	ret = regmap_bulk_read(priv->regmap, NOA1305_REG_ALS_DATA_LSB, &data,
> +			       2);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Lux = count / (<Integration Constant> * <Integration Time>)
> +	 *
> +	 * Integration Constant = 7.7
> +	 * Integration Time in Seconds (currently) = 800ms
> +	 */
> +	return (le16_to_cpu(data) * 100) / (77 * 8);
IIO has always had a preference for passing such conversions over to userspace
which is rather better at doing maths like this (as it has floating point).

Hence, please just return le16_to_cpu(data) and provide the scale value
using the IIO_CHAN_INFO_SCALE element of read_raw.  Use the fractional
form to let the core code work out the decimal value to put out for you.

> +}
> +
> +static const struct iio_chan_spec noa1305_channels[] = {
> +	{
> +		.type = IIO_LIGHT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +	}
> +};
> +
> +static int noa1305_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	int ret = -EINVAL;
> +	struct noa1305_priv *priv = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (chan->type) {
> +		case IIO_LIGHT:
> +			ret = noa1305_measure(priv);
> +			if (ret < 0)
> +				return ret;
> +			*val = ret;
> +			ret = IIO_VAL_INT;
Given there is no cleanup to be done after this, direct
return preferred. 

			return IIO_VAL_INT;
> +			break;
> +		default:
			return -EINVAL;
> +			break;
> +		}
> +		break;
> +	default:
		return -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info noa1305_info = {
> +	.read_raw = noa1305_read_raw,
> +};
> +
> +static bool noa1305_writable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case NOA1305_REG_POWER_CONTROL:
> +	case NOA1305_REG_RESET:
> +	case NOA1305_REG_INTEGRATION_TIME:
> +	case NOA1305_REG_INT_SELECT:
> +	case NOA1305_REG_INT_THRESH_LSB:
> +	case NOA1305_REG_INT_THRESH_MSB:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static const struct regmap_config noa1305_regmap_config = {
> +	.name = NOA1305_DRIVER_NAME,
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = NOA1305_REG_DEVICE_ID_MSB,
> +	.writeable_reg = noa1305_writable_reg,
> +};
> +
> +static void noa1305_reg_remove(void *data)
> +{
> +	struct noa1305_priv *priv = data;
> +
> +	regulator_disable(priv->vin_reg);
> +}
> +
> +static int noa1305_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct noa1305_priv *priv;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	__le16 data;
> +	unsigned int dev_id;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &noa1305_regmap_config);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&client->dev, "Regmap initialization failed.\n");
> +		return PTR_ERR(regmap);
> +	}
> +
> +	priv = iio_priv(indio_dev);
> +
> +	priv->vin_reg = devm_regulator_get(&client->dev, "vin");
> +	if (IS_ERR(priv->vin_reg)) {
> +		dev_err(&client->dev, "get regulator vin failed\n");
> +		return PTR_ERR(priv->vin_reg);
> +	}
> +
> +	ret = regulator_enable(priv->vin_reg);
> +	if (ret) {
> +		dev_err(&client->dev, "enable regulator vin failed\n");
> +		return ret;
> +	}
> +
> +	ret = devm_add_action_or_reset(&client->dev, noa1305_reg_remove, priv);
> +	if (ret) {
> +		dev_err(&client->dev, "addition of devm action failed\n");
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, indio_dev);
> +	priv->client = client;
> +	priv->regmap = regmap;
> +
> +	ret = regmap_bulk_read(regmap, NOA1305_REG_DEVICE_ID_LSB, &data, 2);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "ID reading failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	dev_id = le16_to_cpu(data);
> +	if (dev_id != NOA1305_DEVICE_ID) {
> +		dev_err(&client->dev, "Unknown device ID: 0x%x\n", dev_id);
> +		ret = -ENODEV;
> +		return ret;

return -ENODEV;

> +	}
> +
> +	ret = regmap_write(regmap, NOA1305_REG_POWER_CONTROL,
> +			   NOA1305_POWER_CONTROL_ON);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Enabling power control failed\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_write(regmap, NOA1305_REG_RESET, NOA1305_RESET_RESET);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Device reset failed\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_write(regmap, NOA1305_REG_INTEGRATION_TIME,
> +			   NOA1305_INTEGR_TIME_800MS);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Setting integration time failed\n");
> +		return ret;
> +	}
> +
> +	ret = regmap_write(regmap, NOA1305_REG_INT_SELECT,
> +			   NOA1305_INT_SEL_INACTIVE);

This surprised me so I went and found the datasheet.  It would be unusual
to come out of reset with interrupts on and indeed the datasheet lists
the default as this setting.   Is this working around something you've
seen in reality, or just being (overly) careful?  If the second, don't do
so.

Note however, that if anyone adds regcache support at some point they
will have to manage the condition where writing the reset changes the
values of other registers.

> +	if (ret < 0) {
> +		dev_err(&client->dev, "Disabling interrupts failed\n");
> +		return ret;
> +	}
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &noa1305_info;
> +	indio_dev->channels = noa1305_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(noa1305_channels);
> +	indio_dev->name = NOA1305_DRIVER_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_iio_device_register(&client->dev, indio_dev);
> +	if (ret) {
> +		dev_err(&client->dev, "registering device failed\n");
> +		return ret;

Drop this return ret as we do it anyway below.  One of the static
analysers tends to catch this one, so we normally get follow up
patches very quickly after a driver hits linux next. I'd rather tidy
it up now!

> +	}
> +
> +	return ret;
> +}
> +
> +static int noa1305_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct noa1305_priv *priv = iio_priv(indio_dev);
> +
> +	regulator_disable(priv->vin_reg);

This shouldn't be needed now.  Automated cleanup should be calling
the callback you registered with devm_add_action_or_reset.

As such you can get rid of the remove function entirely as it
has nothing to do.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id noa1305_of_match[] = {
> +	{ .compatible = "onnn,noa1305" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, noa1305_of_match);
> +
> +static const struct i2c_device_id noa1305_ids[] = {
> +	{ "noa1305", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, noa1305_id);
> +
> +static struct i2c_driver noa1305_driver = {
> +	.driver = {
> +		.name		= NOA1305_DRIVER_NAME,
> +		.of_match_table	= noa1305_of_match,
> +	},
> +	.probe		= noa1305_probe,
> +	.remove		= noa1305_remove,
> +	.id_table	= noa1305_ids,
> +};
> +
> +module_i2c_driver(noa1305_driver);
> +
> +MODULE_AUTHOR("Sergei Miroshnichenko <sergeimir@emcraft.com>");
> +MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.com");
> +MODULE_DESCRIPTION("ON Semiconductor NOA1305 ambient light sensor");
> +MODULE_LICENSE("GPL");

^ permalink raw reply

* Re: [PATCH v2 2/2] ARM: dts: imx6ul-kontron-n6310: Add Kontron i.MX6UL N6310 SoM and boards
From: Krzysztof Kozlowski @ 2019-07-27 12:57 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, NXP Linux Team,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-kernel,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Schrempf Frieder
In-Reply-To: <CAOMZO5BPT5Bj+gbgsq+bW5x_NToWqUtz8vmOOS9LyZg5J+CfHQ@mail.gmail.com>

On Fri, Jul 26, 2019 at 02:54:20PM -0300, Fabio Estevam wrote:
> Hi Krzysztof,
> 
> On Fri, Jul 26, 2019 at 3:17 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
> > diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
> > index 7294ac36f4c0..afb61a55e26f 100644
> > --- a/Documentation/devicetree/bindings/arm/fsl.yaml
> > +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
> > @@ -161,6 +161,10 @@ properties:
> >          items:
> >            - enum:
> >                - fsl,imx6ul-14x14-evk      # i.MX6 UltraLite 14x14 EVK Board
> > +              - kontron,n6310-som         # Kontron N6310 SOM
> > +              - kontron,n6310-s           # Kontron N6310 S Board
> > +              - kontron,n6310-s-43        # Kontron N6310 S 43 Board
> > +              - kontron,n6310-s-50        # Kontron N6310 S 50 Board
> 
> These entries should be:
>        imx6ul-kontron-n6310-s.dtb
>        imx6ul-kontron-n6310-s-43.dtb
>        imx6ul-kontron-n6310-s-50.dtb

OK

(I'll apply all your suggestions without and just reply here where there
is some discussion)

> 
> > +       panel {
> > +               compatible = "admatec,t043c004800272t2a";
> 
> I do not find this binding documented.

Because they are not... I mentioned it in commit msg - there are no
driver and bindings for them. I put them for completness of HW
description.

> 
> > +&i2c4 {
> > +       gt911@5d {
> 
> Node names should be generic according to the devicetree spec, so:
> 
> touchscreen@5d
> 
> > +               compatible = "goodix,gt928";
> > +               reg = <0x5d>;
> > +               pinctrl-names = "default";
> > +               pinctrl-0 = <&pinctrl_cap_touch>;
> > +               interrupt-parent = <&gpio5>;
> > +               interrupts = <6 8>;
> 
> It would be better to use a laber to indicate the irq type:
> 
> interrupts = <6 IRQ_TYPE_LEVEL_LOW>;

Indeed.

> 
> > +               reset-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>;
> > +               irq-gpios = <&gpio5 6 GPIO_ACTIVE_HIGH>;
> 
> Active high?
> 
> Above you use "interrupts = <6 8>;" which means IRQ_TYPE_LEVEL_LOW.

Yes, it is confusing but it looks correct. The driver does not use the
GPIO flag so ACTIVE_HIGH or any other setting does not have effect.
Driver uses this pin (as active high) after disabling the interrupts as
an additional reset pin during resume. After this additional reset, it
serves back as interrupt pin.

> 
> > +       };
> > +};
> > +
> > +&iomuxc {
> 
> We tend to prefer putting iomuxc as the last node.
> 
> > +       pinctrl_lcdif_dat: lcdifdatgrp {
> > +               fsl,pins = <
> > +                       MX6UL_PAD_LCD_DATA00__LCDIF_DATA00      0x79
> > +                       MX6UL_PAD_LCD_DATA01__LCDIF_DATA01      0x79
> > +                       MX6UL_PAD_LCD_DATA02__LCDIF_DATA02      0x79
> > +                       MX6UL_PAD_LCD_DATA03__LCDIF_DATA03      0x79
> > +                       MX6UL_PAD_LCD_DATA04__LCDIF_DATA04      0x79
> > +                       MX6UL_PAD_LCD_DATA05__LCDIF_DATA05      0x79
> > +                       MX6UL_PAD_LCD_DATA06__LCDIF_DATA06      0x79
> > +                       MX6UL_PAD_LCD_DATA07__LCDIF_DATA07      0x79
> > +                       MX6UL_PAD_LCD_DATA08__LCDIF_DATA08      0x79
> > +                       MX6UL_PAD_LCD_DATA09__LCDIF_DATA09      0x79
> > +                       MX6UL_PAD_LCD_DATA10__LCDIF_DATA10      0x79
> > +                       MX6UL_PAD_LCD_DATA11__LCDIF_DATA11      0x79
> > +                       MX6UL_PAD_LCD_DATA12__LCDIF_DATA12      0x79
> > +                       MX6UL_PAD_LCD_DATA13__LCDIF_DATA13      0x79
> > +                       MX6UL_PAD_LCD_DATA14__LCDIF_DATA14      0x79
> > +                       MX6UL_PAD_LCD_DATA15__LCDIF_DATA15      0x79
> > +                       MX6UL_PAD_LCD_DATA16__LCDIF_DATA16      0x79
> > +                       MX6UL_PAD_LCD_DATA17__LCDIF_DATA17      0x79
> > +                       MX6UL_PAD_LCD_DATA18__LCDIF_DATA18      0x79
> > +                       MX6UL_PAD_LCD_DATA19__LCDIF_DATA19      0x79
> > +                       MX6UL_PAD_LCD_DATA20__LCDIF_DATA20      0x79
> > +                       MX6UL_PAD_LCD_DATA21__LCDIF_DATA21      0x79
> > +                       MX6UL_PAD_LCD_DATA22__LCDIF_DATA22      0x79
> > +                       MX6UL_PAD_LCD_DATA23__LCDIF_DATA23      0x79
> > +               >;
> > +       };
> > +
> > +       pinctrl_lcdif_ctrl: lcdifctrlgrp {
> > +               fsl,pins = <
> > +                       MX6UL_PAD_LCD_CLK__LCDIF_CLK            0x79
> > +                       MX6UL_PAD_LCD_ENABLE__LCDIF_ENABLE      0x79
> > +                       MX6UL_PAD_LCD_HSYNC__LCDIF_HSYNC        0x79
> > +                       MX6UL_PAD_LCD_VSYNC__LCDIF_VSYNC        0x79
> > +                       MX6UL_PAD_LCD_RESET__LCDIF_RESET        0x79
> > +               >;
> > +       };
> > +
> > +       pinctrl_cap_touch: captouchgrp {
> > +               fsl,pins = <
> > +                       MX6UL_PAD_SNVS_TAMPER6__GPIO5_IO06      0x1b0b0 /* Touch Interrupt */
> > +                       MX6UL_PAD_SNVS_TAMPER7__GPIO5_IO07      0x1b0b0 /* Touch Reset */
> > +                       MX6UL_PAD_SNVS_TAMPER8__GPIO5_IO08      0x1b0b0 /* Touch Wake */
> > +               >;
> > +       };
> > +
> > +       pinctrl_pwm7: pwm7grp {
> > +               fsl,pins = <
> > +                       MX6UL_PAD_CSI_VSYNC__PWM7_OUT           0x110b0
> > +               >;
> > +       };
> > +};
> > +
> > +&lcdif {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_lcdif_dat
> > +                    &pinctrl_lcdif_ctrl>;
> 
> Could fit into a single line.
> 
> > +       panel {
> > +               compatible = "admatec,t070p133t0s301";
> 
> Same here. Undocumented binding.
> 
> > +               backlight = <&backlight>;
> > +
> > +               port {
> > +                       panel_in: endpoint {
> > +                               remote-endpoint = <&display_out>;
> > +                       };
> > +               };
> > +       };
> > +};
> > +
> > +&i2c4 {
> > +       gt911@5d {
> 
> Same comments as previously apply.
> 
> > +
> > +       regulators {
> 
> No need to have this regulators indent level.
> 
> > +               reg_3v3: regulator1 {
> 
> You can place this one directly. The preferred format is:
> 
> reg_3v3: regulator-reg-3v3 {
> 
> > +&ecspi1 {
> > +       fsl,spi-num-chipselects = <1>;
> 
> This property is obsoleted. Please remove it.
> 
> > +       cs-gpios = <&gpio4 26 GPIO_ACTIVE_HIGH>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_ecspi1>;
> > +       status = "okay";
> > +
> > +       fram@0 {
> 
> Generic name please. eeprom@0
> 
> > +               compatible = "atmel,at25";
> 
> Please use the recommended compatible scheme as per
> Documentation/devicetree/bindings/eeprom/at25.txt.
> 
> > +               reg = <0>;
> > +               spi-max-frequency = <20000000>;
> > +               spi-cpha;
> > +               spi-cpol;
> > +               pagesize = <1>;
> > +               size = <8192>;
> > +               address-width = <16>;
> > +       };
> > +};
> 
> 
> > +&usbotg1 {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_usbotg1>;
> > +       dr_mode = "otg";
> > +       status = "okay";
> 
> We prefer to put the 'status' property as the last one.
> 
> > +       srp-disable;
> > +       hnp-disable;
> > +       adp-disable;
> > +       vbus-supply = <&reg_usb_otg1_vbus>;
> > +};
> 
> > +/ {
> > +       model = "Kontron N6310 SOM";
> > +       compatible = "kontron,n6310-som", "fsl,imx6ul";
> > +
> > +       memory@80000000 {
> 
> device_type = "memory"; is missing here.
> 
> > +               reg = <0x80000000 0x10000000>;
> > +       };
> > +};
> > +
> > +&cpu0 {
> > +       clock-frequency = <528000000>;
> 
> Is this one really needed?

I'll check.

> 
> > +&ecspi2 {
> > +       cs-gpios = <&gpio4 22 GPIO_ACTIVE_HIGH>;
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_ecspi2>;
> > +       status = "okay";
> > +
> > +       flash: mx25v80@0 {
> 
> spi-flash@0
> 
> > +&qspi {
> > +       pinctrl-names = "default";
> > +       pinctrl-0 = <&pinctrl_qspi>;
> > +       status = "okay";
> > +
> > +       flash0: w25m02gv@0 {
> 
> generic name, please.

Thanks for review!

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 2/4] crypto: amlogic: Add crypto accelerator for amlogic GXL
From: Martin Blumenstingl @ 2019-07-27 12:17 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: davem, herbert, khilman, mark.rutland, robh+dt, devicetree,
	baylibre-upstreaming, linux-kernel, linux-crypto, linux-amlogic,
	linux-arm-kernel
In-Reply-To: <1564083776-20540-3-git-send-email-clabbe@baylibre.com>

Hi Corentin,

it's great to see you working on this :)

On Thu, Jul 25, 2019 at 9:45 PM Corentin Labbe <clabbe@baylibre.com> wrote:
>
> This patch adds support for the amlogic GXL cryptographic offloader present
> on GXL SoCs.
>
> This driver supports AES cipher in CBC/ECB mode.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  drivers/crypto/Kconfig                  |   2 +
>  drivers/crypto/Makefile                 |   1 +
>  drivers/crypto/amlogic/Kconfig          |  24 ++
>  drivers/crypto/amlogic/Makefile         |   2 +
>  drivers/crypto/amlogic/amlogic-cipher.c | 358 ++++++++++++++++++++++++
>  drivers/crypto/amlogic/amlogic-core.c   | 326 +++++++++++++++++++++
>  drivers/crypto/amlogic/amlogic.h        | 172 ++++++++++++
>  7 files changed, 885 insertions(+)
>  create mode 100644 drivers/crypto/amlogic/Kconfig
>  create mode 100644 drivers/crypto/amlogic/Makefile
>  create mode 100644 drivers/crypto/amlogic/amlogic-cipher.c
>  create mode 100644 drivers/crypto/amlogic/amlogic-core.c
>  create mode 100644 drivers/crypto/amlogic/amlogic.h
there are two different crypto IPs on Amlogic SoCs:
- GXL and newer use the "BLKMV" crypto IP
- GXBB, Meson8/Meson8b/Meson8m2 (and probably older SoCs) use the
"NDMA" crypto IP

personally I think it makes sense to either have the IP name (blkmv)
or SoC name (GXL) in the file or directory names as well as being
consistent with that in the Kconfig option names

(I have no experience with the crypto framework so I cannot comment on
the driver implementation itself)


Martin

^ permalink raw reply

* [PATCH v3 4/4] MIPS: lantiq: update the clock alias' for the mainline PCIe PHY driver
From: Martin Blumenstingl @ 2019-07-27 12:04 UTC (permalink / raw)
  To: linux-mips, devicetree, john, kishon, paul.burton, ralf
  Cc: robh+dt, linux-kernel, hauke, mark.rutland, ms,
	Martin Blumenstingl
In-Reply-To: <20190727120415.15859-1-martin.blumenstingl@googlemail.com>

The mainline PCIe PHY driver has it's own devicetree node. Update the
clock alias so the mainline driver finds the clocks.

The first PCIe PHY is located at 0x1f106800 and exists on VRX200, ARX300
and GRX390.
The second PCIe PHY is located at 0x1f700400 and exists on ARX300 and
GRX390.
The third PCIe PHY is located at 0x1f106a00 and exists onl on GRX390.
Lantiq's board support package (called "UGW") names these registers
"PDI".

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 arch/mips/lantiq/xway/sysctrl.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/mips/lantiq/xway/sysctrl.c b/arch/mips/lantiq/xway/sysctrl.c
index b4323b2214e2..156a95ac5c72 100644
--- a/arch/mips/lantiq/xway/sysctrl.c
+++ b/arch/mips/lantiq/xway/sysctrl.c
@@ -468,14 +468,14 @@ void __init ltq_soc_init(void)
 		clkdev_add_pmu("1f203018.usb2-phy", "phy", 1, 2, PMU_ANALOG_USB0_P);
 		clkdev_add_pmu("1f203034.usb2-phy", "phy", 1, 2, PMU_ANALOG_USB1_P);
 		/* rc 0 */
-		clkdev_add_pmu("1d900000.pcie", "phy", 1, 2, PMU_ANALOG_PCIE0_P);
+		clkdev_add_pmu("1f106800.phy", "phy", 1, 2, PMU_ANALOG_PCIE0_P);
 		clkdev_add_pmu("1d900000.pcie", "msi", 1, 1, PMU1_PCIE_MSI);
-		clkdev_add_pmu("1d900000.pcie", "pdi", 1, 1, PMU1_PCIE_PDI);
+		clkdev_add_pmu("1f106800.phy", "pdi", 1, 1, PMU1_PCIE_PDI);
 		clkdev_add_pmu("1d900000.pcie", "ctl", 1, 1, PMU1_PCIE_CTL);
 		/* rc 1 */
-		clkdev_add_pmu("19000000.pcie", "phy", 1, 2, PMU_ANALOG_PCIE1_P);
+		clkdev_add_pmu("1f700400.phy", "phy", 1, 2, PMU_ANALOG_PCIE1_P);
 		clkdev_add_pmu("19000000.pcie", "msi", 1, 1, PMU1_PCIE1_MSI);
-		clkdev_add_pmu("19000000.pcie", "pdi", 1, 1, PMU1_PCIE1_PDI);
+		clkdev_add_pmu("1f700400.phy", "pdi", 1, 1, PMU1_PCIE1_PDI);
 		clkdev_add_pmu("19000000.pcie", "ctl", 1, 1, PMU1_PCIE1_CTL);
 	}
 
@@ -499,9 +499,9 @@ void __init ltq_soc_init(void)
 		clkdev_add_pmu("1e101000.usb", "otg", 1, 0, PMU_USB0);
 		clkdev_add_pmu("1e106000.usb", "otg", 1, 0, PMU_USB1);
 		/* rc 2 */
-		clkdev_add_pmu("1a800000.pcie", "phy", 1, 2, PMU_ANALOG_PCIE2_P);
+		clkdev_add_pmu("1f106a00.pcie", "phy", 1, 2, PMU_ANALOG_PCIE2_P);
 		clkdev_add_pmu("1a800000.pcie", "msi", 1, 1, PMU1_PCIE2_MSI);
-		clkdev_add_pmu("1a800000.pcie", "pdi", 1, 1, PMU1_PCIE2_PDI);
+		clkdev_add_pmu("1f106a00.pcie", "pdi", 1, 1, PMU1_PCIE2_PDI);
 		clkdev_add_pmu("1a800000.pcie", "ctl", 1, 1, PMU1_PCIE2_CTL);
 		clkdev_add_pmu("1e10b308.eth", NULL, 0, 0, PMU_SWITCH | PMU_PPE_DP);
 		clkdev_add_pmu("1da00000.usif", "NULL", 1, 0, PMU_USIF);
@@ -526,10 +526,10 @@ void __init ltq_soc_init(void)
 		clkdev_add_pmu("1e101000.usb", "otg", 1, 0, PMU_USB0 | PMU_AHBM);
 		clkdev_add_pmu("1f203034.usb2-phy", "phy", 1, 0, PMU_USB1_P);
 		clkdev_add_pmu("1e106000.usb", "otg", 1, 0, PMU_USB1 | PMU_AHBM);
-		clkdev_add_pmu("1d900000.pcie", "phy", 1, 1, PMU1_PCIE_PHY);
+		clkdev_add_pmu("1f106800.phy", "phy", 1, 1, PMU1_PCIE_PHY);
 		clkdev_add_pmu("1d900000.pcie", "bus", 1, 0, PMU_PCIE_CLK);
 		clkdev_add_pmu("1d900000.pcie", "msi", 1, 1, PMU1_PCIE_MSI);
-		clkdev_add_pmu("1d900000.pcie", "pdi", 1, 1, PMU1_PCIE_PDI);
+		clkdev_add_pmu("1f106800.phy", "pdi", 1, 1, PMU1_PCIE_PDI);
 		clkdev_add_pmu("1d900000.pcie", "ctl", 1, 1, PMU1_PCIE_CTL);
 		clkdev_add_pmu(NULL, "ahb", 1, 0, PMU_AHBM | PMU_AHBS);
 
-- 
2.22.0

^ permalink raw reply related

* [PATCH v3 3/4] phy: enable compile-testing for the Lantiq PHY drivers
From: Martin Blumenstingl @ 2019-07-27 12:04 UTC (permalink / raw)
  To: linux-mips, devicetree, john, kishon, paul.burton, ralf
  Cc: robh+dt, linux-kernel, hauke, mark.rutland, ms,
	Martin Blumenstingl
In-Reply-To: <20190727120415.15859-1-martin.blumenstingl@googlemail.com>

Unconditionally include the lantiq subdirectory in the phy Makefile.

All drivers in there have their dependencies maintained. One of these
(optional) dependencies is COMPILE_TEST, however this can only be
evaluated when Kconfig scans the lantiq subdirectory.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/phy/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 0d9fddc498a6..c96a1afc95bd 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -10,7 +10,6 @@ obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
 obj-$(CONFIG_PHY_PISTACHIO_USB)		+= phy-pistachio-usb.o
 obj-$(CONFIG_ARCH_SUNXI)		+= allwinner/
 obj-$(CONFIG_ARCH_MESON)		+= amlogic/
-obj-$(CONFIG_LANTIQ)			+= lantiq/
 obj-$(CONFIG_ARCH_MEDIATEK)		+= mediatek/
 obj-$(CONFIG_ARCH_RENESAS)		+= renesas/
 obj-$(CONFIG_ARCH_ROCKCHIP)		+= rockchip/
@@ -19,6 +18,7 @@ obj-y					+= broadcom/	\
 					   cadence/	\
 					   freescale/	\
 					   hisilicon/	\
+					   lantiq/	\
 					   marvell/	\
 					   motorola/	\
 					   mscc/	\
-- 
2.22.0

^ permalink raw reply related

* [PATCH v3 2/4] phy: lantiq: vrx200-pcie: add a driver for the Lantiq VRX200 PCIe PHY
From: Martin Blumenstingl @ 2019-07-27 12:04 UTC (permalink / raw)
  To: linux-mips, devicetree, john, kishon, paul.burton, ralf
  Cc: robh+dt, linux-kernel, hauke, mark.rutland, ms,
	Martin Blumenstingl
In-Reply-To: <20190727120415.15859-1-martin.blumenstingl@googlemail.com>

The Lantiq VRX200 SoCs embed a PCIe PHY in the "sram" bus. Unlike most
other IP blocks on this SoC the register values are only 16-bit wide.
Like other IP blocks on this SoC the register values are in big endian.

The PHY embeds a PLL which can be configured in various modes. Only the
36MHz mode is supported for now, the other modes can be implemented when
there's a board which actually needs them. OpenWrt uses the out-of-tree
vendor driver and all supported boards there only need the 36MHz mode.

There are two input clocks:
- the "pdi" clock enables the register access
- the "phy" clock is the clock input and enables the internal PLL

There are two reset lines:
- "phy" resets the PHY itself
- the "pcie" reset line is shared between the PHY and the PCIe
  controller

While the VRX200 SoC has only one PCIe controller and PHY the ARX300
uses two identical PCIe controllers and PHYs which are compatible with
the PCIe controller and PHY on VRX200.
Add a driver for this PHY so PCIe support can be enabled on these SoCs.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/phy/lantiq/Kconfig                  |  11 +
 drivers/phy/lantiq/Makefile                 |   1 +
 drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c | 494 ++++++++++++++++++++
 3 files changed, 506 insertions(+)
 create mode 100644 drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c

diff --git a/drivers/phy/lantiq/Kconfig b/drivers/phy/lantiq/Kconfig
index eb66c857ce25..c4df9709d53f 100644
--- a/drivers/phy/lantiq/Kconfig
+++ b/drivers/phy/lantiq/Kconfig
@@ -2,6 +2,17 @@
 #
 # Phy drivers for Lantiq / Intel platforms
 #
+config PHY_LANTIQ_VRX200_PCIE
+	tristate "Lantiq VRX200/ARX300 PCIe PHY"
+	depends on SOC_TYPE_XWAY || COMPILE_TEST
+	depends on OF && HAS_IOMEM
+	select GENERIC_PHY
+	select REGMAP_MMIO
+	help
+	  Support for the PCIe PHY(s) on the Lantiq / Intel VRX200 and ARX300
+	  family SoCs.
+	  If unsure, say N.
+
 config PHY_LANTIQ_RCU_USB2
 	tristate "Lantiq XWAY SoC RCU based USB PHY"
 	depends on OF && (SOC_TYPE_XWAY || COMPILE_TEST)
diff --git a/drivers/phy/lantiq/Makefile b/drivers/phy/lantiq/Makefile
index 540049039092..7c14eb24ab73 100644
--- a/drivers/phy/lantiq/Makefile
+++ b/drivers/phy/lantiq/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_PHY_LANTIQ_RCU_USB2)	+= phy-lantiq-rcu-usb2.o
+obj-$(CONFIG_PHY_LANTIQ_VRX200_PCIE)	+= phy-lantiq-vrx200-pcie.o
diff --git a/drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c b/drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c
new file mode 100644
index 000000000000..544d64a84cc0
--- /dev/null
+++ b/drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c
@@ -0,0 +1,494 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PCIe PHY driver for Lantiq VRX200 and ARX300 SoCs.
+ *
+ * Copyright (C) 2019 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ *
+ * Based on the BSP (called "UGW") driver:
+ *  Copyright (C) 2009-2015 Lei Chuanhua <chuanhua.lei@lantiq.com>
+ *  Copyright (C) 2016 Intel Corporation
+ *
+ * TODO: PHY modes other than 36MHz (without "SSC")
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include <dt-bindings/phy/phy-lantiq-vrx200-pcie.h>
+
+#define PCIE_PHY_PLL_CTRL1				0x44
+
+#define PCIE_PHY_PLL_CTRL2				0x46
+#define PCIE_PHY_PLL_CTRL2_CONST_SDM_MASK		GENMASK(7, 0)
+#define PCIE_PHY_PLL_CTRL2_CONST_SDM_EN			BIT(8)
+#define PCIE_PHY_PLL_CTRL2_PLL_SDM_EN			BIT(9)
+
+#define PCIE_PHY_PLL_CTRL3				0x48
+#define PCIE_PHY_PLL_CTRL3_EXT_MMD_DIV_RATIO_EN		BIT(1)
+#define PCIE_PHY_PLL_CTRL3_EXT_MMD_DIV_RATIO_MASK	GENMASK(6, 4)
+
+#define PCIE_PHY_PLL_CTRL4				0x4a
+#define PCIE_PHY_PLL_CTRL5				0x4c
+#define PCIE_PHY_PLL_CTRL6				0x4e
+#define PCIE_PHY_PLL_CTRL7				0x50
+#define PCIE_PHY_PLL_A_CTRL1				0x52
+
+#define PCIE_PHY_PLL_A_CTRL2				0x54
+#define PCIE_PHY_PLL_A_CTRL2_LF_MODE_EN			BIT(14)
+
+#define PCIE_PHY_PLL_A_CTRL3				0x56
+#define PCIE_PHY_PLL_A_CTRL3_MMD_MASK			GENMASK(15, 13)
+
+#define PCIE_PHY_PLL_STATUS				0x58
+
+#define PCIE_PHY_TX1_CTRL1				0x60
+#define PCIE_PHY_TX1_CTRL1_FORCE_EN			BIT(3)
+#define PCIE_PHY_TX1_CTRL1_LOAD_EN			BIT(4)
+
+#define PCIE_PHY_TX1_CTRL2				0x62
+#define PCIE_PHY_TX1_CTRL3				0x64
+#define PCIE_PHY_TX1_A_CTRL1				0x66
+#define PCIE_PHY_TX1_A_CTRL2				0x68
+#define PCIE_PHY_TX1_MOD1				0x6a
+#define PCIE_PHY_TX1_MOD2				0x6c
+#define PCIE_PHY_TX1_MOD3				0x6e
+
+#define PCIE_PHY_TX2_CTRL1				0x70
+#define PCIE_PHY_TX2_CTRL1_LOAD_EN			BIT(4)
+
+#define PCIE_PHY_TX2_CTRL2				0x72
+#define PCIE_PHY_TX2_A_CTRL1				0x76
+#define PCIE_PHY_TX2_A_CTRL2				0x78
+#define PCIE_PHY_TX2_MOD1				0x7a
+#define PCIE_PHY_TX2_MOD2				0x7c
+#define PCIE_PHY_TX2_MOD3				0x7e
+
+#define PCIE_PHY_RX1_CTRL1				0xa0
+#define PCIE_PHY_RX1_CTRL1_LOAD_EN			BIT(1)
+
+#define PCIE_PHY_RX1_CTRL2				0xa2
+#define PCIE_PHY_RX1_CDR				0xa4
+#define PCIE_PHY_RX1_EI					0xa6
+#define PCIE_PHY_RX1_A_CTRL				0xaa
+
+struct ltq_vrx200_pcie_phy_priv {
+	struct phy			*phy;
+	unsigned int			mode;
+	struct device			*dev;
+	struct regmap			*phy_regmap;
+	struct regmap			*rcu_regmap;
+	struct clk			*pdi_clk;
+	struct clk			*phy_clk;
+	struct reset_control		*phy_reset;
+	struct reset_control		*pcie_reset;
+	u32				rcu_ahb_endian_offset;
+	u32				rcu_ahb_endian_big_endian_mask;
+};
+
+static void ltq_vrx200_pcie_phy_common_setup(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+
+	/* PLL Setting */
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_A_CTRL1, 0x120e);
+
+	/* increase the bias reference voltage */
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_A_CTRL2, 0x39d7);
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_A_CTRL3, 0x0900);
+
+	/* Endcnt */
+	regmap_write(priv->phy_regmap, PCIE_PHY_RX1_EI, 0x0004);
+	regmap_write(priv->phy_regmap, PCIE_PHY_RX1_A_CTRL, 0x6803);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_TX1_CTRL1,
+			   PCIE_PHY_TX1_CTRL1_FORCE_EN,
+			   PCIE_PHY_TX1_CTRL1_FORCE_EN);
+
+	/* predrv_ser_en */
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX1_A_CTRL2, 0x0706);
+
+	/* ctrl_lim */
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX1_CTRL3, 0x1fff);
+
+	/* ctrl */
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX1_A_CTRL1, 0x0810);
+
+	/* predrv_ser_en */
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_TX2_A_CTRL2, 0x7f00,
+			   0x4700);
+
+	/* RTERM */
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX1_CTRL2, 0x2e00);
+
+	/* Improved 100MHz clock output  */
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX2_CTRL2, 0x3096);
+	regmap_write(priv->phy_regmap, PCIE_PHY_TX2_A_CTRL2, 0x4707);
+
+	/* Reduced CDR BW to avoid glitches */
+	regmap_write(priv->phy_regmap, PCIE_PHY_RX1_CDR, 0x0235);
+}
+
+static void pcie_phy_36mhz_mode_setup(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_CTRL3,
+			   PCIE_PHY_PLL_CTRL3_EXT_MMD_DIV_RATIO_EN, 0x0000);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_CTRL3,
+			   PCIE_PHY_PLL_CTRL3_EXT_MMD_DIV_RATIO_MASK, 0x0000);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_CTRL2,
+			   PCIE_PHY_PLL_CTRL2_PLL_SDM_EN,
+			   PCIE_PHY_PLL_CTRL2_PLL_SDM_EN);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_CTRL2,
+			   PCIE_PHY_PLL_CTRL2_CONST_SDM_EN,
+			   PCIE_PHY_PLL_CTRL2_CONST_SDM_EN);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_A_CTRL3,
+			   PCIE_PHY_PLL_A_CTRL3_MMD_MASK,
+			   FIELD_PREP(PCIE_PHY_PLL_A_CTRL3_MMD_MASK, 0x1));
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_A_CTRL2,
+			   PCIE_PHY_PLL_A_CTRL2_LF_MODE_EN, 0x0000);
+
+	/* const_sdm */
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_CTRL1, 0x38e4);
+
+	regmap_update_bits(priv->phy_regmap, PCIE_PHY_PLL_CTRL2,
+			   PCIE_PHY_PLL_CTRL2_CONST_SDM_MASK,
+			   FIELD_PREP(PCIE_PHY_PLL_CTRL2_CONST_SDM_MASK,
+				      0xee));
+
+	/* pllmod */
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_CTRL7, 0x0002);
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_CTRL6, 0x3a04);
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_CTRL5, 0xfae3);
+	regmap_write(priv->phy_regmap, PCIE_PHY_PLL_CTRL4, 0x1b72);
+}
+
+static int ltq_vrx200_pcie_phy_wait_for_pll(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+	unsigned int tmp;
+	int ret;
+
+	ret = regmap_read_poll_timeout(priv->phy_regmap, PCIE_PHY_PLL_STATUS,
+				       tmp, ((tmp & 0x0070) == 0x0070), 10,
+				       10000);
+	if (ret) {
+		dev_err(priv->dev, "PLL Link timeout, PLL status = 0x%04x\n",
+			tmp);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void ltq_vrx200_pcie_phy_apply_workarounds(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+	static const struct reg_default slices[] =  {
+		{
+			.reg = PCIE_PHY_TX1_CTRL1,
+			.def = PCIE_PHY_TX1_CTRL1_LOAD_EN,
+		},
+		{
+			.reg = PCIE_PHY_TX2_CTRL1,
+			.def = PCIE_PHY_TX2_CTRL1_LOAD_EN,
+		},
+		{
+			.reg = PCIE_PHY_RX1_CTRL1,
+			.def = PCIE_PHY_RX1_CTRL1_LOAD_EN,
+		}
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(slices); i++) {
+		/* enable load_en */
+		regmap_update_bits(priv->phy_regmap, slices[i].reg,
+				   slices[i].def, slices[i].def);
+
+		udelay(1);
+
+		/* disable load_en */
+		regmap_update_bits(priv->phy_regmap, slices[i].reg,
+				   slices[i].def, 0x0);
+	}
+
+	for (i = 0; i < 5; i++) {
+		/* TX2 modulation */
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX2_MOD1, 0x1ffe);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX2_MOD2, 0xfffe);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX2_MOD3, 0x0601);
+		usleep_range(1000, 2000);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX2_MOD3, 0x0001);
+
+		/* TX1 modulation */
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX1_MOD1, 0x1ffe);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX1_MOD2, 0xfffe);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX1_MOD3, 0x0601);
+		usleep_range(1000, 2000);
+		regmap_write(priv->phy_regmap, PCIE_PHY_TX1_MOD3, 0x0001);
+	}
+}
+
+static int ltq_vrx200_pcie_phy_init(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	if (of_device_is_big_endian(priv->dev->of_node))
+		regmap_update_bits(priv->rcu_regmap,
+				   priv->rcu_ahb_endian_offset,
+				   priv->rcu_ahb_endian_big_endian_mask,
+				   priv->rcu_ahb_endian_big_endian_mask);
+	else
+		regmap_update_bits(priv->rcu_regmap,
+				   priv->rcu_ahb_endian_offset,
+				   priv->rcu_ahb_endian_big_endian_mask, 0x0);
+
+	ret = reset_control_assert(priv->phy_reset);
+	if (ret)
+		goto err;
+
+	udelay(1);
+
+	ret = reset_control_deassert(priv->phy_reset);
+	if (ret)
+		goto err;
+
+	udelay(1);
+
+	ret = reset_control_deassert(priv->pcie_reset);
+	if (ret)
+		goto err_assert_phy_reset;
+
+	/* Make sure PHY PLL is stable */
+	usleep_range(20, 40);
+
+	return 0;
+
+err_assert_phy_reset:
+	reset_control_assert(priv->phy_reset);
+err:
+	return ret;
+}
+
+static int ltq_vrx200_pcie_phy_exit(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	ret = reset_control_assert(priv->pcie_reset);
+	if (ret)
+		return ret;
+
+	ret = reset_control_assert(priv->phy_reset);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int ltq_vrx200_pcie_phy_power_on(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+	int ret;
+
+	/* Enable PDI to access PCIe PHY register */
+	ret = clk_prepare_enable(priv->pdi_clk);
+	if (ret)
+		goto err;
+
+	/* Configure PLL and PHY clock */
+	ltq_vrx200_pcie_phy_common_setup(phy);
+
+	pcie_phy_36mhz_mode_setup(phy);
+
+	/* Enable the PCIe PHY and make PLL setting take effect */
+	ret = clk_prepare_enable(priv->phy_clk);
+	if (ret)
+		goto err_disable_pdi_clk;
+
+	/* Check if we are in "startup ready" status */
+	if (ltq_vrx200_pcie_phy_wait_for_pll(phy) != 0)
+		goto err_disable_phy_clk;
+
+	ltq_vrx200_pcie_phy_apply_workarounds(phy);
+
+	return 0;
+
+err_disable_phy_clk:
+	clk_disable_unprepare(priv->phy_clk);
+err_disable_pdi_clk:
+	clk_disable_unprepare(priv->pdi_clk);
+err:
+	return ret;
+}
+
+static int ltq_vrx200_pcie_phy_power_off(struct phy *phy)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = phy_get_drvdata(phy);
+
+	clk_disable_unprepare(priv->phy_clk);
+	clk_disable_unprepare(priv->pdi_clk);
+
+	return 0;
+}
+
+static struct phy_ops ltq_vrx200_pcie_phy_ops = {
+	.init		= ltq_vrx200_pcie_phy_init,
+	.exit		= ltq_vrx200_pcie_phy_exit,
+	.power_on	= ltq_vrx200_pcie_phy_power_on,
+	.power_off	= ltq_vrx200_pcie_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static struct phy *ltq_vrx200_pcie_phy_xlate(struct device *dev,
+					     struct of_phandle_args *args)
+{
+	struct ltq_vrx200_pcie_phy_priv *priv = dev_get_drvdata(dev);
+	unsigned int mode;
+
+	if (args->args_count != 1) {
+		dev_err(dev, "invalid number of arguments\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	mode = args->args[0];
+
+	switch (mode) {
+	case LANTIQ_PCIE_PHY_MODE_36MHZ:
+		priv->mode = mode;
+		break;
+
+	case LANTIQ_PCIE_PHY_MODE_25MHZ:
+	case LANTIQ_PCIE_PHY_MODE_25MHZ_SSC:
+	case LANTIQ_PCIE_PHY_MODE_36MHZ_SSC:
+	case LANTIQ_PCIE_PHY_MODE_100MHZ:
+	case LANTIQ_PCIE_PHY_MODE_100MHZ_SSC:
+		dev_err(dev, "PHY mode not implemented yet: %u\n", mode);
+		return ERR_PTR(-EINVAL);
+
+	default:
+		dev_err(dev, "invalid PHY mode %u\n", mode);
+		return ERR_PTR(-EINVAL);
+	};
+
+	return priv->phy;
+}
+
+static int ltq_vrx200_pcie_phy_probe(struct platform_device *pdev)
+{
+	static const struct regmap_config regmap_config = {
+		.reg_bits = 8,
+		.val_bits = 16,
+		.reg_stride = 2,
+		.max_register = PCIE_PHY_RX1_A_CTRL,
+	};
+	struct ltq_vrx200_pcie_phy_priv *priv;
+	struct device *dev = &pdev->dev;
+	struct phy_provider *provider;
+	struct resource *res;
+	void __iomem *base;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	priv->phy_regmap = devm_regmap_init_mmio(dev, base, &regmap_config);
+	if (IS_ERR(priv->phy_regmap))
+		return PTR_ERR(priv->phy_regmap);
+
+	priv->rcu_regmap = syscon_regmap_lookup_by_phandle(dev->of_node,
+							   "lantiq,rcu");
+	if (IS_ERR(priv->rcu_regmap))
+		return PTR_ERR(priv->rcu_regmap);
+
+	ret = device_property_read_u32(dev, "lantiq,rcu-endian-offset",
+				       &priv->rcu_ahb_endian_offset);
+	if (ret) {
+		dev_err(dev,
+			"failed to parse the 'lantiq,rcu-endian-offset' property\n");
+		return ret;
+	}
+
+	ret = device_property_read_u32(dev, "lantiq,rcu-big-endian-mask",
+				       &priv->rcu_ahb_endian_big_endian_mask);
+	if (ret) {
+		dev_err(dev,
+			"failed to parse the 'lantiq,rcu-big-endian-mask' property\n");
+		return ret;
+	}
+
+	priv->pdi_clk = devm_clk_get(dev, "pdi");
+	if (IS_ERR(priv->pdi_clk))
+		return PTR_ERR(priv->pdi_clk);
+
+	priv->phy_clk = devm_clk_get(dev, "phy");
+	if (IS_ERR(priv->phy_clk))
+		return PTR_ERR(priv->phy_clk);
+
+	priv->phy_reset = devm_reset_control_get_exclusive(dev, "phy");
+	if (IS_ERR(priv->phy_reset))
+		return PTR_ERR(priv->phy_reset);
+
+	priv->pcie_reset = devm_reset_control_get_shared(dev, "pcie");
+	if (IS_ERR(priv->pcie_reset))
+		return PTR_ERR(priv->pcie_reset);
+
+	priv->dev = dev;
+
+	priv->phy = devm_phy_create(dev, dev->of_node,
+				    &ltq_vrx200_pcie_phy_ops);
+	if (IS_ERR(priv->phy)) {
+		dev_err(dev, "failed to create PHY\n");
+		return PTR_ERR(priv->phy);
+	}
+
+	phy_set_drvdata(priv->phy, priv);
+	dev_set_drvdata(dev, priv);
+
+	provider = devm_of_phy_provider_register(dev,
+						 ltq_vrx200_pcie_phy_xlate);
+
+	return PTR_ERR_OR_ZERO(provider);
+}
+
+static const struct of_device_id ltq_vrx200_pcie_phy_of_match[] = {
+	{ .compatible = "lantiq,vrx200-pcie-phy", },
+	{ .compatible = "lantiq,arx300-pcie-phy", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ltq_vrx200_pcie_phy_of_match);
+
+static struct platform_driver ltq_vrx200_pcie_phy_driver = {
+	.probe	= ltq_vrx200_pcie_phy_probe,
+	.driver = {
+		.name	= "ltq-vrx200-pcie-phy",
+		.of_match_table	= ltq_vrx200_pcie_phy_of_match,
+	}
+};
+module_platform_driver(ltq_vrx200_pcie_phy_driver);
+
+MODULE_AUTHOR("Martin Blumenstingl <martin.blumenstingl@googlemail.com>");
+MODULE_DESCRIPTION("Lantiq VRX200 and ARX300 PCIe PHY driver");
+MODULE_LICENSE("GPL v2");
-- 
2.22.0

^ permalink raw reply related

* [PATCH v3 1/4] dt-bindings: phy: add binding for the Lantiq VRX200 and ARX300 PCIe PHYs
From: Martin Blumenstingl @ 2019-07-27 12:04 UTC (permalink / raw)
  To: linux-mips, devicetree, john, kishon, paul.burton, ralf
  Cc: robh+dt, linux-kernel, hauke, mark.rutland, ms,
	Martin Blumenstingl, Rob Herring
In-Reply-To: <20190727120415.15859-1-martin.blumenstingl@googlemail.com>

Add the bindings for the PCIe PHY on Lantiq VRX200 and ARX300 SoCs.
The IP block contains settings for the PHY and a PLL.
The PLL mode is configurable through a dedicated #phy-cell in .dts.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/phy/lantiq,vrx200-pcie-phy.yaml  | 95 +++++++++++++++++++
 .../dt-bindings/phy/phy-lantiq-vrx200-pcie.h  | 11 +++
 2 files changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml
 create mode 100644 include/dt-bindings/phy/phy-lantiq-vrx200-pcie.h

diff --git a/Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml
new file mode 100644
index 000000000000..8a56a8526cef
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/lantiq,vrx200-pcie-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Lantiq VRX200 and ARX300 PCIe PHY Device Tree Bindings
+
+maintainers:
+  - Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+
+properties:
+  "#phy-cells":
+    const: 1
+    description: selects the PHY mode as defined in <dt-bindings/phy/phy-lantiq-vrx200-pcie.h>
+
+  compatible:
+    enum:
+      - lantiq,vrx200-pcie-phy
+      - lantiq,arx300-pcie-phy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: PHY module clock
+      - description: PDI register clock
+
+  clock-names:
+    items:
+      - const: phy
+      - const: pdi
+
+  resets:
+    items:
+      - description: exclusive PHY reset line
+      - description: shared reset line between the PCIe PHY and PCIe controller
+
+  resets-names:
+    items:
+      - const: phy
+      - const: pcie
+
+  lantiq,rcu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the RCU syscon
+
+  lantiq,rcu-endian-offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: the offset of the endian registers for this PHY instance in the RCU syscon
+
+  lantiq,rcu-big-endian-mask:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: the mask to set the PDI (PHY) registers for this PHY instance to big endian
+
+  big-endian:
+    description: Configures the PDI (PHY) registers in big-endian mode
+    type: boolean
+
+  little-endian:
+    description: Configures the PDI (PHY) registers in big-endian mode
+    type: boolean
+
+required:
+  - "#phy-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - lantiq,rcu
+  - lantiq,rcu-endian-offset
+  - lantiq,rcu-big-endian-mask
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie0_phy: phy@106800 {
+        compatible = "lantiq,vrx200-pcie-phy";
+        reg = <0x106800 0x100>;
+        lantiq,rcu = <&rcu0>;
+        lantiq,rcu-endian-offset = <0x4c>;
+        lantiq,rcu-big-endian-mask = <0x80>; /* bit 7 */
+        big-endian;
+        clocks = <&pmu 32>, <&pmu 36>;
+        clock-names = "phy", "pdi";
+        resets = <&reset0 12 24>, <&reset0 22 22>;
+        reset-names = "phy", "pcie";
+        #phy-cells = <1>;
+    };
+
+...
diff --git a/include/dt-bindings/phy/phy-lantiq-vrx200-pcie.h b/include/dt-bindings/phy/phy-lantiq-vrx200-pcie.h
new file mode 100644
index 000000000000..95a7896356d6
--- /dev/null
+++ b/include/dt-bindings/phy/phy-lantiq-vrx200-pcie.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2019 Martin Blumenstingl <martin.blumenstingl@googlemail.com>
+ */
+
+#define LANTIQ_PCIE_PHY_MODE_25MHZ		0
+#define LANTIQ_PCIE_PHY_MODE_25MHZ_SSC		1
+#define LANTIQ_PCIE_PHY_MODE_36MHZ		2
+#define LANTIQ_PCIE_PHY_MODE_36MHZ_SSC		3
+#define LANTIQ_PCIE_PHY_MODE_100MHZ		4
+#define LANTIQ_PCIE_PHY_MODE_100MHZ_SSC		5
-- 
2.22.0

^ permalink raw reply related

* [PATCH v3 0/4] Lantiq VRX200/ARX300 PCIe PHY driver
From: Martin Blumenstingl @ 2019-07-27 12:04 UTC (permalink / raw)
  To: linux-mips, devicetree, john, kishon, paul.burton, ralf
  Cc: robh+dt, linux-kernel, hauke, mark.rutland, ms,
	Martin Blumenstingl

Various Lantiq (now Intel) SoCs contain one or more PCIe controllers
and PHYs.
This adds a driver for the PCIe PHYs found on the Lantiq VRX200 and
ARX300 SoCs. GRX390 should also be supported as far as I can tell,
but I don't have any of these devices to further verify that.

I have tested this PCIe PHY driver with the out-of-tree PCIe controller
driver in OpenWrt: [0]

dependencies for this series:
none

patches 1-3 should go through the PHY tree
patch 4 should go through the mips tree

I am aware that this series is too late for the v5.3 development cycle.
Getting review comments is still appreciated so this can be queued early
in the v5.4 development cycle.


Changes since v2 at [2]:
- added Rob's Reviewed-by to the dt-bindings patch (thank you!)

Changes since v1 at [1]:
- many thanks to Rob for giving me many hints regarding the .yaml bindings!
- update the .yaml binding license to (GPL-2.0-only OR BSD-2-Clause)
- changed the property lantiq,rcu to type phandle
- add the optional big-endian and little-endian boolean properties
- use numeric values for the clock phandles in the example to make the
  dt_binding_check build happy
- replaced two mdelay(1); with usleep_range(1000, 2000); in patch #2
  (spotted and reported by Hauke off-list)


[0] https://github.com/xdarklight/openwrt/commits/lantiq-mainline-pcie-phy-20190702
[1] https://patchwork.kernel.org/cover/11028797/
[2] https://patchwork.kernel.org/cover/11031421/


Martin Blumenstingl (4):
  dt-bindings: phy: add binding for the Lantiq VRX200 and ARX300 PCIe
    PHYs
  phy: lantiq: vrx200-pcie: add a driver for the Lantiq VRX200 PCIe PHY
  phy: enable compile-testing for the Lantiq PHY drivers
  MIPS: lantiq: update the clock alias' for the mainline PCIe PHY driver

 .../bindings/phy/lantiq,vrx200-pcie-phy.yaml  |  95 ++++
 arch/mips/lantiq/xway/sysctrl.c               |  16 +-
 drivers/phy/Makefile                          |   2 +-
 drivers/phy/lantiq/Kconfig                    |  11 +
 drivers/phy/lantiq/Makefile                   |   1 +
 drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c   | 494 ++++++++++++++++++
 .../dt-bindings/phy/phy-lantiq-vrx200-pcie.h  |  11 +
 7 files changed, 621 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml
 create mode 100644 drivers/phy/lantiq/phy-lantiq-vrx200-pcie.c
 create mode 100644 include/dt-bindings/phy/phy-lantiq-vrx200-pcie.h

-- 
2.22.0

^ permalink raw reply

* [PATCH] dt-bindings: imx: i.MX8MN: Use space instead of tab
From: Guido Günther @ 2019-07-27 11:29 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

This fixes 'make dt_binding_check'

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
Seen on next-20190726

 Documentation/devicetree/bindings/clock/imx8mn-clock.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/imx8mn-clock.yaml b/Documentation/devicetree/bindings/clock/imx8mn-clock.yaml
index 454c5b44b2ee..622f3658bd9f 100644
--- a/Documentation/devicetree/bindings/clock/imx8mn-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/imx8mn-clock.yaml
@@ -71,7 +71,7 @@ examples:
         compatible = "fixed-clock";
         #clock-cells = <0>;
         clock-frequency = <32768>;
-	clock-output-names = "osc_32k";
+        clock-output-names = "osc_32k";
     };
 
     osc_24m: clock-osc-24m {
-- 
2.20.1

^ permalink raw reply related

* Re: [GIT PULL] Devicetree fixes for 5.3-rc, take 2
From: Greg Kroah-Hartman @ 2019-07-27 11:06 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linus Torvalds, Frank Rowand, devicetree,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAL_JsqJLB4q6wqTOX0oXAGQF4wuZ0irNT8nmpFEmuUKjvv38BQ@mail.gmail.com>

On Fri, Jul 26, 2019 at 06:03:50PM -0600, Rob Herring wrote:
> Hi Linus,
> 
> Please pull some more DT fixes for 5.3. The nvmem changes would
> typically go thru Greg's tree, but they were missed in the merge
> window and I've been unable to get a response (partly because Srinivas
> is out on vacation it appears).

No objection from me for this.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 6/6] arm64: dts: allwinner: h6: Add PWM node
From: Maxime Ripard @ 2019-07-27 10:51 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: thierry.reding, wens, robh+dt, mark.rutland, linux-pwm,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20190726184045.14669-7-jernej.skrabec@siol.net>

On Fri, Jul 26, 2019 at 08:40:45PM +0200, Jernej Skrabec wrote:
> Allwinner H6 PWM is similar to that in A20 except that it has additional
> bus clock and reset line.
>
> Note that first PWM channel is connected to output pin and second
> channel is used internally, as a clock source to AC200 co-packaged chip.
> This means that any combination of these two channels can be used and
> thus it doesn't make sense to add pinctrl nodes at this point.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> index e8bed58e7246..c1abd805cfdc 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
> @@ -229,6 +229,16 @@
>  			status = "disabled";
>  		};
>
> +		pwm: pwm@300a000 {
> +			compatible = "allwinner,sun50i-h6-pwm";
> +			reg = <0x0300a000 0x400>;
> +			clocks = <&osc24M>, <&ccu CLK_BUS_PWM>;
> +			clock-names = "pwm", "bus";

We always have the bus clock first, so I'd really like to keep
that. We also usually use mod for the second clock, and not the name
of the device itself.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH 5/6] pwm: sun4i: Add support to output source clock directly
From: Maxime Ripard @ 2019-07-27 10:50 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: thierry.reding, wens, robh+dt, mark.rutland, linux-pwm,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20190726184045.14669-6-jernej.skrabec@siol.net>

On Fri, Jul 26, 2019 at 08:40:44PM +0200, Jernej Skrabec wrote:
> PWM core has an option to bypass whole logic and output unchanged source
> clock as PWM output. This is achieved by enabling bypass bit.
>
> Note that when bypass is enabled, no other setting has any meaning, not
> even enable bit.
>
> This mode of operation is needed to achieve high enough frequency to
> serve as clock source for AC200 chip, which is integrated into same
> package as H6 SoC.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

It doesn't seem to be available on the A10 (at least) though. The A13
seem to have it, so you should probably check that, and make that
conditional to the compatible if not available on all of them.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH 3/6] pwm: sun4i: Add a quirk for bus clock
From: Maxime Ripard @ 2019-07-27 10:46 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: thierry.reding, wens, robh+dt, mark.rutland, linux-pwm,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20190726184045.14669-4-jernej.skrabec@siol.net>

Hi,

On Fri, Jul 26, 2019 at 08:40:42PM +0200, Jernej Skrabec wrote:
> H6 PWM core needs bus clock to be enabled in order to work.
>
> Add a quirk for it.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  drivers/pwm/pwm-sun4i.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> index 1b7be8fbde86..7d3ac3f2dc3f 100644
> --- a/drivers/pwm/pwm-sun4i.c
> +++ b/drivers/pwm/pwm-sun4i.c
> @@ -72,6 +72,7 @@ static const u32 prescaler_table[] = {
>  };
>
>  struct sun4i_pwm_data {
> +	bool has_bus_clock;
>  	bool has_prescaler_bypass;
>  	bool has_reset;
>  	unsigned int npwm;
> @@ -79,6 +80,7 @@ struct sun4i_pwm_data {
>
>  struct sun4i_pwm_chip {
>  	struct pwm_chip chip;
> +	struct clk *bus_clk;
>  	struct clk *clk;
>  	struct reset_control *rst;
>  	void __iomem *base;
> @@ -382,6 +384,16 @@ static int sun4i_pwm_probe(struct platform_device *pdev)
>  		reset_control_deassert(pwm->rst);
>  	}
>
> +	if (pwm->data->has_bus_clock) {
> +		pwm->bus_clk = devm_clk_get(&pdev->dev, "bus");
> +		if (IS_ERR(pwm->bus_clk)) {
> +			ret = PTR_ERR(pwm->bus_clk);
> +			goto err_bus;
> +		}
> +
> +		clk_prepare_enable(pwm->bus_clk);
> +	}
> +

The patch itself looks fine, but you should clarify which clock is
being used by the old driver.

My guess is that the "new" clock is actually the mod one, while the
old one was both the clock of the register interface (bus) and the
clock of the PWM generation logic (mod).

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH 2/6] pwm: sun4i: Add a quirk for reset line
From: Maxime Ripard @ 2019-07-27 10:42 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: thierry.reding, wens, robh+dt, mark.rutland, linux-pwm,
	devicetree, linux-arm-kernel, linux-kernel, linux-sunxi
In-Reply-To: <20190726184045.14669-3-jernej.skrabec@siol.net>

On Fri, Jul 26, 2019 at 08:40:41PM +0200, Jernej Skrabec wrote:
> H6 PWM core needs deasserted reset line in order to work.
>
> Add a quirk for it.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply

* Re: [PATCH 1/6] dt-bindings: pwm: allwinner: Add H6 PWM description
From: Maxime Ripard @ 2019-07-27 10:42 UTC (permalink / raw)
  To: Jernej Skrabec
  Cc: mark.rutland, linux-pwm, devicetree, linux-sunxi, linux-kernel,
	robh+dt, wens, thierry.reding, linux-arm-kernel
In-Reply-To: <20190726184045.14669-2-jernej.skrabec@siol.net>

Hi,

On Fri, Jul 26, 2019 at 08:40:40PM +0200, Jernej Skrabec wrote:
> H6 PWM block is basically the same as A20 PWM, except that it also has
> bus clock and reset line which needs to be handled accordingly.
>
> Expand Allwinner PWM binding with H6 PWM specifics.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> ---
>  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 36 ++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> index 0ac52f83a58c..deca5d81802f 100644
> --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> @@ -30,13 +30,47 @@ properties:
>        - items:
>            - const: allwinner,sun50i-h5-pwm
>            - const: allwinner,sun5i-a13-pwm
> +      - const: allwinner,sun50i-h6-pwm
>
>    reg:
>      maxItems: 1
>
> -  clocks:
> +  # Even though it only applies to subschemas under the conditionals,
> +  # not listing them here will trigger a warning because of the
> +  # additionalsProperties set to false.
> +  clocks: true
> +  clock-names: true
> +  resets:
>      maxItems: 1
>
> +allOf:
> +  - if:

There's only one condition, so you don't really need the allOf.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ 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