* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Boris Brezillon @ 2019-08-09 15:25 UTC (permalink / raw)
To: Tomer Maimon
Cc: Mark Brown, Rob Herring, Mark Rutland, Vignesh Raghavendra,
Boris Brezillon, Avi Fishman, Tali Perry, Patrick Venture,
Nancy Yuen, Benjamin Fair, linux-spi, devicetree,
OpenBMC Maillist, Linux Kernel Mailing List
In-Reply-To: <CAP6Zq1iW0C0FDOoqmn5r_xk5HQFWw+GgLfeapvt-8mB50N2Vvg@mail.gmail.com>
On Fri, 9 Aug 2019 18:26:23 +0300
Tomer Maimon <tmaimon77@gmail.com> wrote:
> Hi Boris,
>
> Thanks a lot for your comment.
>
> On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <boris.brezillon@collabora.com>
> wrote:
>
> > On Thu, 8 Aug 2019 16:14:48 +0300
> > Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> >
> > > +
> > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > > + .exec_op = npcm_fiu_exec_op,
> >
> > No npcm_supports_op()? That's suspicious, especially after looking at
> > the npcm_fiu_exec_op() (and the functions called from there) where the
> > requested ->buswidth seems to be completely ignored...
> >
> > Sorry but I do not fully understand it, do you mean a support for the
> buswidth?
> If yes it been done in the UMA functions as follow:
>
> uma_cfg |= ilog2(op->cmd.buswidth);
> uma_cfg |= ilog2(op->addr.buswidth) <<
> NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
> uma_cfg |= ilog2(op->data.buswidth) <<
> NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
> uma_cfg |= op->addr.nbytes << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
> regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, op->addr.val);
>
Hm, the default supports_op() implementation might be just fine for
your use case. But there's one thing you still need to check: the
number of addr cycles (or address size as you call it in this driver).
Looks like your IP is limited to 4 address cycles, if I'm right, you
should reject any operation that have op->addr.nbytes > 4. I also
wonder if there's a limitation on the data size you can have on a
single transfer. If there's one you should implement ->adjust_op() too.
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-09 15:26 UTC (permalink / raw)
To: Boris Brezillon
Cc: Mark Brown, Rob Herring, Mark Rutland, Vignesh Raghavendra,
Boris Brezillon, Avi Fishman, Tali Perry, Patrick Venture,
Nancy Yuen, Benjamin Fair, linux-spi, devicetree,
OpenBMC Maillist, Linux Kernel Mailing List
In-Reply-To: <20190808173232.4d79d698@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
Hi Boris,
Thanks a lot for your comment.
On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <boris.brezillon@collabora.com>
wrote:
> On Thu, 8 Aug 2019 16:14:48 +0300
> Tomer Maimon <tmaimon77@gmail.com> wrote:
>
>
> > +
> > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > + .exec_op = npcm_fiu_exec_op,
>
> No npcm_supports_op()? That's suspicious, especially after looking at
> the npcm_fiu_exec_op() (and the functions called from there) where the
> requested ->buswidth seems to be completely ignored...
>
> Sorry but I do not fully understand it, do you mean a support for the
buswidth?
If yes it been done in the UMA functions as follow:
uma_cfg |= ilog2(op->cmd.buswidth);
uma_cfg |= ilog2(op->addr.buswidth) <<
NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
uma_cfg |= ilog2(op->data.buswidth) <<
NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
uma_cfg |= op->addr.nbytes << NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR, op->addr.val);
> + .dirmap_create = npcm_fiu_dirmap_create,
> > + .dirmap_read = npcm_fiu_direct_read,
> > + .dirmap_write = npcm_fiu_direct_write,
> > +};
> > +
>
Thanks,
Tomer
[-- Attachment #2: Type: text/html, Size: 2294 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-09 15:31 UTC (permalink / raw)
To: Mark Brown
Cc: Rob Herring, Mark Rutland, Vignesh Raghavendra, Boris Brezillon,
Avi Fishman, Tali Perry, Patrick Venture, Nancy Yuen,
Benjamin Fair, linux-spi, devicetree, OpenBMC Maillist,
Linux Kernel Mailing List
In-Reply-To: <20190808185522.GJ3795@sirena.co.uk>
[-- Attachment #1: Type: text/plain, Size: 893 bytes --]
Hi Mark,
Thanks for your prompt reply.
I will like to send another patch with support for the spi_mem_op->addr
field.
I am going to vacation until 25/08 I will send the new patch (V3) as soon I
will be back.
Thanks for your support,
Tomer
On Thu, 8 Aug 2019 at 21:55, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Aug 08, 2019 at 06:37:06PM +0300, Tomer Maimon wrote:
>
> > for example in our driver we modify the access type (singe, dual or quad)
> > according the op->addr.buswidth
> > for example in the npcm_fiu_set_drd function.
>
> > regmap_update_bits(fiu->regmap, NPCM_FIU_DRD_CFG,
> > NPCM_FIU_DRD_CFG_ACCTYPE,
> > ilog2(op->addr.buswidth) <<
> > NPCM_FIU_DRD_ACCTYPE_SHIFT);
>
> > we also modify it in the UMA R/W functions.
>
> Ah, it's only for the flash functions - that's fine.
>
[-- Attachment #2: Type: text/html, Size: 1448 bytes --]
^ permalink raw reply
* Re: [PATCH v3 12/21] ARM: dts: imx6-apalis: Add touchscreens used on Toradex eval boards
From: Marcel Ziswiler @ 2019-08-09 15:35 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-13-philippe.schenker@toradex.com>
Hi Philippe
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This commit adds the touchscreens from Toradex so one can enable it.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>
> ---
>
> Changes in v3:
> - Fix commit title to "...imx6-apalis:..."
>
> Changes in v2:
> - Deleted touchrevolution downstream stuff
> - Use generic node name
> - Put a better comment in there
>
> arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts | 31
> +++++++++++++++++++
> arch/arm/boot/dts/imx6q-apalis-eval.dts | 13 ++++++++
> arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts | 13 ++++++++
> arch/arm/boot/dts/imx6q-apalis-ixora.dts | 13 ++++++++
> 4 files changed, 70 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> index 9a5d6c94cca4..763fb5e90bd3 100644
> --- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> @@ -168,6 +168,21 @@
> &i2c3 {
> status = "okay";
>
> + /*
> + * Touchscreen is using SODIMM 28/30, also used for PWM<B>,
> PWM<C>,
> + * aka pwm2, pwm3. so if you enable touchscreen, disable the
> pwms
> + */
> + touchscreen@4a {
> + compatible = "atmel,maxtouch";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pcap_1>;
> + reg = <0x4a>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <9 IRQ_TYPE_EDGE_FALLING>; /*
> SODIMM 28 */
> + reset-gpios = <&gpio2 10 GPIO_ACTIVE_HIGH>; /*
> SODIMM 30 */
> + status = "disabled";
> + };
> +
> /* M41T0M6 real time clock on carrier board */
> rtc_i2c: rtc@68 {
> compatible = "st,m41t0";
> @@ -175,6 +190,22 @@
> };
> };
>
> +&iomuxc {
> + pinctrl_pcap_1: pcap-1 {
> + fsl,pins = <
> + MX6QDL_PAD_GPIO_9__GPIO1_IO09 0x1b0b0 /*
> SODIMM 28 */
> + MX6QDL_PAD_SD4_DAT2__GPIO2_IO10 0x1b0b0 /*
> SODIMM 30 */
> + >;
> + };
What exactly are the above which get used further up vs. the below
which do not seem to get used anywhere?
> + pinctrl_mxt_ts: mxt-ts {
> + fsl,pins = <
> + MX6QDL_PAD_EIM_CS1__GPIO2_IO24 0x130b0 /*
> SODIMM 107 */
> + MX6QDL_PAD_SD2_DAT1__GPIO1_IO14 0x130b0 /*
> SODIMM 106 */
> + >;
> + };
> +};
> +
> &ipu1_di0_disp0 {
> remote-endpoint = <&lcd_display_in>;
> };
> diff --git a/arch/arm/boot/dts/imx6q-apalis-eval.dts
> b/arch/arm/boot/dts/imx6q-apalis-eval.dts
> index 0edd3043d9c1..4665e15b196d 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-eval.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-eval.dts
> @@ -167,6 +167,19 @@
> &i2c1 {
> status = "okay";
>
> + /*
> + * Touchscreen is using SODIMM 28/30, also used for PWM<B>,
> PWM<C>,
> + * aka pwm2, pwm3. so if you enable touchscreen, disable the
> pwms
> + */
> + touchscreen@4a {
> + compatible = "atmel,maxtouch";
> + reg = <0x4a>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
Wouldn't above two pins also need resp. pinctrl entries?
> + status = "disabled";
> + };
> +
> pcie-switch@58 {
> compatible = "plx,pex8605";
> reg = <0x58>;
> diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> index b94bb687be6b..a3fa04a97d81 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-ixora-v1.1.dts
> @@ -172,6 +172,19 @@
> &i2c1 {
> status = "okay";
>
> + /*
> + * Touchscreen is using SODIMM 28/30, also used for PWM<B>,
> PWM<C>,
> + * aka pwm2, pwm3. so if you enable touchscreen, disable the
> pwms
> + */
> + touchscreen@4a {
> + compatible = "atmel,maxtouch";
> + reg = <0x4a>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
Ditto.
> + status = "disabled";
> + };
> +
> /* M41T0M6 real time clock on carrier board */
> rtc_i2c: rtc@68 {
> compatible = "st,m41t0";
> diff --git a/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> index 302fd6adc8a7..5ba49d0f4880 100644
> --- a/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> +++ b/arch/arm/boot/dts/imx6q-apalis-ixora.dts
> @@ -171,6 +171,19 @@
> &i2c1 {
> status = "okay";
>
> + /*
> + * Touchscreen is using SODIMM 28/30, also used for PWM<B>,
> PWM<C>,
> + * aka pwm2, pwm3. so if you enable touchscreen, disable the
> pwms
> + */
> + touchscreen@4a {
> + compatible = "atmel,maxtouch";
> + reg = <0x4a>;
> + interrupt-parent = <&gpio6>;
> + interrupts = <10 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 9 GPIO_ACTIVE_HIGH>; /* SODIMM 13
> */
Ditto.
> + status = "disabled";
> + };
> +
> eeprom@50 {
> compatible = "atmel,24c02";
> reg = <0x50>;
Cheers
Marcel
^ permalink raw reply
* Re: [PATCH v3 13/21] ARM: dts: imx6-colibri: Add missing pinmuxing to Toradex eval board
From: Marcel Ziswiler @ 2019-08-09 15:36 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-14-philippe.schenker@toradex.com>
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This patch adds some missing pinmuxing that is in the colibri
> standard to the dts.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Commit title
>
> arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> index 763fb5e90bd3..e7a2d8c3b2d4 100644
> --- a/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> +++ b/arch/arm/boot/dts/imx6dl-colibri-eval-v3.dts
> @@ -191,6 +191,14 @@
> };
>
> &iomuxc {
> + pinctrl-names = "default";
> + pinctrl-0 = <
> + &pinctrl_weim_gpio_1 &pinctrl_weim_gpio_2
> + &pinctrl_weim_gpio_3 &pinctrl_weim_gpio_4
> + &pinctrl_weim_gpio_5 &pinctrl_weim_gpio_6
> + &pinctrl_usbh_oc_1 &pinctrl_usbc_id_1
> + >;
> +
> pinctrl_pcap_1: pcap-1 {
> fsl,pins = <
> MX6QDL_PAD_GPIO_9__GPIO1_IO09 0x1b0b0 /*
> SODIMM 28 */
^ permalink raw reply
* Re: [PATCH v3 14/21] ARM: dts: imx6ull-colibri: Add sleep mode to fec
From: Marcel Ziswiler @ 2019-08-09 15:38 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-imx@nxp.com, s.hauer@pengutronix.de, kernel@pengutronix.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190807082556.5013-15-philippe.schenker@toradex.com>
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> Do not change the clock as the power for this phy is switched
> with that clock.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6ull-colibri.dtsi | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index d56728f03c35..1019ce69a242 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -62,8 +62,9 @@
> };
>
> &fec2 {
> - pinctrl-names = "default";
> + pinctrl-names = "default", "sleep";
> pinctrl-0 = <&pinctrl_enet2>;
> + pinctrl-1 = <&pinctrl_enet2_sleep>;
> phy-mode = "rmii";
> phy-handle = <ðphy1>;
> status = "okay";
> @@ -220,6 +221,21 @@
> >;
> };
>
> + pinctrl_enet2_sleep: enet2sleepgrp {
> + fsl,pins = <
> + MX6UL_PAD_GPIO1_IO06__GPIO1_IO06 0x0
> + MX6UL_PAD_GPIO1_IO07__GPIO1_IO07 0x0
> + MX6UL_PAD_ENET2_RX_DATA0__GPIO2_IO08 0x0
> + MX6UL_PAD_ENET2_RX_DATA1__GPIO2_IO09 0x0
> + MX6UL_PAD_ENET2_RX_EN__GPIO2_IO10 0x0
> + MX6UL_PAD_ENET2_RX_ER__GPIO2_IO15 0x0
> + MX6UL_PAD_ENET2_TX_CLK__ENET2_REF_CLK2 0x400
> 1b031
> + MX6UL_PAD_ENET2_TX_DATA0__GPIO2_IO11 0x0
> + MX6UL_PAD_ENET2_TX_DATA1__GPIO2_IO12 0x0
> + MX6UL_PAD_ENET2_TX_EN__GPIO2_IO13 0x0
> + >;
> + };
> +
> pinctrl_ecspi1_cs: ecspi1-cs-grp {
> fsl,pins = <
> MX6UL_PAD_LCD_DATA21__GPIO3_IO26 0x000a0
^ permalink raw reply
* Re: [PATCH v3 15/21] ARM: dts: imx6ull-colibri: reduce v_batt current in power off
From: Marcel Ziswiler @ 2019-08-09 15:40 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-16-philippe.schenker@toradex.com>
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
>
> Reduce the current drawn from VCC_BATT when the main power on the 3V3
> pins to the module are switched off.
>
> This switches off SoC internal pull resistors which are provided on
> the
> module for TAMPER7 and TAMPER9 SoC pin and switches on a pull down
> instead of a pullup for the USBC_DET module pin (TAMPER2).
>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6ull-colibri.dtsi | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index 1019ce69a242..1f112ec55e5c 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -533,19 +533,19 @@
>
> pinctrl_snvs_ad7879_int: snvs-ad7879-int-grp { /* TOUCH
> Interrupt */
> fsl,pins = <
> - MX6ULL_PAD_SNVS_TAMPER7__GPIO5_IO07 0x1b0
> b0
> + MX6ULL_PAD_SNVS_TAMPER7__GPIO5_IO07 0x100
> b0
> >;
> };
>
> pinctrl_snvs_reg_sd: snvs-reg-sd-grp {
> fsl,pins = <
> - MX6ULL_PAD_SNVS_TAMPER9__GPIO5_IO09 0x400
> 1b8b0
> + MX6ULL_PAD_SNVS_TAMPER9__GPIO5_IO09 0x400
> 100b0
> >;
> };
>
> pinctrl_snvs_usbc_det: snvs-usbc-det-grp {
> fsl,pins = <
> - MX6ULL_PAD_SNVS_TAMPER2__GPIO5_IO02 0x1b0
> b0
> + MX6ULL_PAD_SNVS_TAMPER2__GPIO5_IO02 0x130
> b0
> >;
> };
^ permalink raw reply
* Re: [PATCH v3 16/21] ARM: dts: imx6ull-colibri: Add watchdog
From: Marcel Ziswiler @ 2019-08-09 15:43 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-17-philippe.schenker@toradex.com>
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This patch adds the watchdog to the imx6ull-colibri devicetree
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index 1f112ec55e5c..e3220298dd6f 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -199,6 +199,12 @@
> assigned-clock-rates = <0>, <198000000>;
> };
>
> +&wdog1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_wdog>;
> + fsl,ext-reset-output;
> +};
> +
> &iomuxc {
> pinctrl_can_int: canint-grp {
> fsl,pins = <
> @@ -506,6 +512,12 @@
> MX6UL_PAD_GPIO1_IO03__OSC32K_32K_OUT 0x14
> >;
> };
> +
> + pinctrl_wdog: wdog-grp {
> + fsl,pins = <
> + MX6UL_PAD_LCD_RESET__WDOG1_WDOG_ANY 0x30b0
> + >;
> + };
> };
>
> &iomuxc_snvs {
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-09 15:47 UTC (permalink / raw)
To: Boris Brezillon
Cc: Mark Brown, Rob Herring, Mark Rutland, Vignesh Raghavendra,
Boris Brezillon, Avi Fishman, Tali Perry, Patrick Venture,
Nancy Yuen, Benjamin Fair, linux-spi, devicetree,
OpenBMC Maillist, Linux Kernel Mailing List
In-Reply-To: <20190809172557.346e7c41@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]
On Fri, 9 Aug 2019 at 18:26, Boris Brezillon <boris.brezillon@collabora.com>
wrote:
> On Fri, 9 Aug 2019 18:26:23 +0300
> Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> > Hi Boris,
> >
> > Thanks a lot for your comment.
> >
> > On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <
> boris.brezillon@collabora.com>
> > wrote:
> >
> > > On Thu, 8 Aug 2019 16:14:48 +0300
> > > Tomer Maimon <tmaimon77@gmail.com> wrote:
> > >
> > >
> > > > +
> > > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > > > + .exec_op = npcm_fiu_exec_op,
> > >
> > > No npcm_supports_op()? That's suspicious, especially after looking at
> > > the npcm_fiu_exec_op() (and the functions called from there) where the
> > > requested ->buswidth seems to be completely ignored...
> > >
> > > Sorry but I do not fully understand it, do you mean a support for the
> > buswidth?
> > If yes it been done in the UMA functions as follow:
> >
> > uma_cfg |= ilog2(op->cmd.buswidth);
> > uma_cfg |= ilog2(op->addr.buswidth) <<
> > NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
> > uma_cfg |= ilog2(op->data.buswidth) <<
> > NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
> > uma_cfg |= op->addr.nbytes <<
> NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
> > regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR,
> op->addr.val);
> >
>
> Hm, the default supports_op() implementation might be just fine for
> your use case. But there's one thing you still need to check: the
> number of addr cycles (or address size as you call it in this driver).
> Looks like your IP is limited to 4 address cycles, if I'm right, you
> should reject any operation that have op->addr.nbytes > 4. I also
>
Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash with
more than 32bit address?
I will add this limitation thanks!
> wonder if there's a limitation on the data size you can have on a
> single transfer. If there's one you should implement ->adjust_op() too.
>
there is a limitation in a single transfer but I handle it in the
npcm_fiu_manualwrite
function.
Do you suggest to use ->adjust_op() instead?
[-- Attachment #2: Type: text/html, Size: 3742 bytes --]
^ permalink raw reply
* Re: [PATCH v3 17/21] ARM: dts: imx6ull: improve can templates
From: Marcel Ziswiler @ 2019-08-09 15:47 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-18-philippe.schenker@toradex.com>
Hi Philippe
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
>
> Add the pinmuxing and a inactive node for flexcan1 on SODIMM 55/63
> and move the inactive flexcan nodes to imx6ull-colibri-eval-v3.dtsi
> where they belong.
>
> Note that this commit does not enable flexcan functionality, but
> rather
> eases the effort needed to do so.
>
> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 12 ++++++++++++
> arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi | 2 +-
> arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi | 2 +-
> arch/arm/boot/dts/imx6ull-colibri.dtsi | 16 ++++++++++++++--
> 4 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> index b6147c76d159..3bee37c75aa6 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -83,6 +83,18 @@
> };
> };
>
> +&can1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_flexcan1>;
> + status = "disabled";
> +};
> +
> +&can2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_flexcan2>;
> + status = "disabled";
> +};
As those don't really have anything to do with the eval board directly,
wouldn't it make more sense to rather move them into the module's dtsi
just like the pin muxing further below?
> &i2c1 {
> status = "okay";
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> index fb213bec4654..95a11b8bcbdb 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-nonwifi.dtsi
> @@ -15,7 +15,7 @@
> &iomuxc {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> - &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6>;
> + &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio6
> &pinctrl_gpio7>;
> };
>
> &iomuxc_snvs {
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> index 038d8c90f6df..a0545431b3dc 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-wifi.dtsi
> @@ -26,7 +26,7 @@
> &iomuxc {
> pinctrl-names = "default";
> pinctrl-0 = <&pinctrl_gpio1 &pinctrl_gpio2 &pinctrl_gpio3
> - &pinctrl_gpio4 &pinctrl_gpio5>;
> + &pinctrl_gpio4 &pinctrl_gpio5 &pinctrl_gpio7>;
>
> };
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> index e3220298dd6f..553d4c1f80e9 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
> @@ -256,6 +256,13 @@
> >;
> };
>
> + pinctrl_flexcan1: flexcan1-grp {
> + fsl,pins = <
> + MX6UL_PAD_ENET1_RX_DATA0__FLEXCAN1_TX 0x1b0
> 20
> + MX6UL_PAD_ENET1_RX_DATA1__FLEXCAN1_RX 0x1b0
> 20
> + >;
> + };
> +
> pinctrl_flexcan2: flexcan2-grp {
> fsl,pins = <
> MX6UL_PAD_ENET1_TX_DATA0__FLEXCAN2_RX 0x1b0
> 20
> @@ -271,8 +278,6 @@
>
> pinctrl_gpio1: gpio1-grp {
> fsl,pins = <
> - MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00 0x74
> /* SODIMM 55 */
> - MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01 0x74
> /* SODIMM 63 */
> MX6UL_PAD_UART3_RX_DATA__GPIO1_IO25 0X14
> /* SODIMM 77 */
> MX6UL_PAD_JTAG_TCK__GPIO1_IO14 0x14
> /* SODIMM 99 */
> MX6UL_PAD_NAND_CE1_B__GPIO4_IO14 0x14 /*
> SODIMM 133 */
> @@ -325,6 +330,13 @@
> >;
> };
>
> + pinctrl_gpio7: gpio7-grp { /* CAN1 */
> + fsl,pins = <
> + MX6UL_PAD_ENET1_RX_DATA0__GPIO2_IO00 0x74
> /* SODIMM 55 */
> + MX6UL_PAD_ENET1_RX_DATA1__GPIO2_IO01 0x74
> /* SODIMM 63 */
> + >;
> + };
> +
> pinctrl_gpmi_nand: gpmi-nand-grp {
> fsl,pins = <
> MX6UL_PAD_NAND_DATA00__RAWNAND_DATA00 0x100
> a9
Cheers
Marcel
^ permalink raw reply
* Re: [PATCH v3 18/21] ARM: dts: imx6ull-colibri: Add general wakeup key used on Colibri
From: Marcel Ziswiler @ 2019-08-09 15:48 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-19-philippe.schenker@toradex.com>
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This adds the possibility to wake the module with an external signal
> as defined in the Colibri standard
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> index 3bee37c75aa6..d3c4809f140e 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -8,6 +8,20 @@
> stdout-path = "serial0:115200n8";
> };
>
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_snvs_gpiokeys>;
> +
> + power {
> + label = "Wake-Up";
> + gpios = <&gpio5 1 GPIO_ACTIVE_HIGH>;
> + linux,code = <KEY_WAKEUP>;
> + debounce-interval = <10>;
> + wakeup-source;
> + };
> + };
> +
> /* fixed crystal dedicated to mcp2515 */
> clk16m: clk16m {
> compatible = "fixed-clock";
^ permalink raw reply
* Re: [PATCH v3 19/21] ARM: dts: imx6/7-colibri: switch dr_mode to otg
From: Marcel Ziswiler @ 2019-08-09 15:50 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-imx@nxp.com, s.hauer@pengutronix.de, kernel@pengutronix.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190807082556.5013-20-philippe.schenker@toradex.com>
Hi Philippe
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> In order for the otg ports, that these modules support, it is needed
> that dr_mode is on otg. Switch to use that feature.
Isn't further extcon integration required for this to truly work?
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> ---
>
> Changes in v3: None
> Changes in v2: None
>
> arch/arm/boot/dts/imx6qdl-colibri.dtsi | 2 +-
> arch/arm/boot/dts/imx7-colibri.dtsi | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> index 9a63debab0b5..6674198346d2 100644
> --- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
> @@ -388,7 +388,7 @@
> &usbotg {
> pinctrl-names = "default";
> disable-over-current;
> - dr_mode = "peripheral";
> + dr_mode = "otg";
> status = "disabled";
> };
>
> diff --git a/arch/arm/boot/dts/imx7-colibri.dtsi
> b/arch/arm/boot/dts/imx7-colibri.dtsi
> index 67f5e0c87fdc..42478f1aa146 100644
> --- a/arch/arm/boot/dts/imx7-colibri.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri.dtsi
> @@ -320,7 +320,7 @@
> };
>
> &usbotg1 {
> - dr_mode = "host";
> + dr_mode = "otg";
> };
>
> &usdhc1 {
Cheers
Marcel
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Boris Brezillon @ 2019-08-09 15:51 UTC (permalink / raw)
To: Tomer Maimon
Cc: Mark Brown, Rob Herring, Mark Rutland, Vignesh Raghavendra,
Boris Brezillon, Avi Fishman, Tali Perry, Patrick Venture,
Nancy Yuen, Benjamin Fair, linux-spi, devicetree,
OpenBMC Maillist, Linux Kernel Mailing List
In-Reply-To: <CAP6Zq1hc0kNHzCE6tcLZdv7NcNWEdn5nh=Wzd8pdbZTuj31Hbg@mail.gmail.com>
On Fri, 9 Aug 2019 18:47:08 +0300
Tomer Maimon <tmaimon77@gmail.com> wrote:
> On Fri, 9 Aug 2019 at 18:26, Boris Brezillon <boris.brezillon@collabora.com>
> wrote:
>
> > On Fri, 9 Aug 2019 18:26:23 +0300
> > Tomer Maimon <tmaimon77@gmail.com> wrote:
> >
> > > Hi Boris,
> > >
> > > Thanks a lot for your comment.
> > >
> > > On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <
> > boris.brezillon@collabora.com>
> > > wrote:
> > >
> > > > On Thu, 8 Aug 2019 16:14:48 +0300
> > > > Tomer Maimon <tmaimon77@gmail.com> wrote:
> > > >
> > > >
> > > > > +
> > > > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > > > > + .exec_op = npcm_fiu_exec_op,
> > > >
> > > > No npcm_supports_op()? That's suspicious, especially after looking at
> > > > the npcm_fiu_exec_op() (and the functions called from there) where the
> > > > requested ->buswidth seems to be completely ignored...
> > > >
> > > > Sorry but I do not fully understand it, do you mean a support for the
> > > buswidth?
> > > If yes it been done in the UMA functions as follow:
> > >
> > > uma_cfg |= ilog2(op->cmd.buswidth);
> > > uma_cfg |= ilog2(op->addr.buswidth) <<
> > > NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
> > > uma_cfg |= ilog2(op->data.buswidth) <<
> > > NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
> > > uma_cfg |= op->addr.nbytes <<
> > NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
> > > regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR,
> > op->addr.val);
> > >
> >
> > Hm, the default supports_op() implementation might be just fine for
> > your use case. But there's one thing you still need to check: the
> > number of addr cycles (or address size as you call it in this driver).
> > Looks like your IP is limited to 4 address cycles, if I'm right, you
> > should reject any operation that have op->addr.nbytes > 4. I also
> >
> Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash with
> more than 32bit address?
spi-mem is not only about spi-nor, it can be used for any kind of
memory (NOR, NAND, SRAM, ...) or even to communicate with an FGPA, so
yes, you have to take care of that.
> I will add this limitation thanks!
>
> > wonder if there's a limitation on the data size you can have on a
> > single transfer. If there's one you should implement ->adjust_op() too.
> >
> there is a limitation in a single transfer but I handle it in the
> npcm_fiu_manualwrite
> function.
> Do you suggest to use ->adjust_op() instead?
Yes, should be exposed through ->adjust_op() => the caller needs to
know when a new operation (one containing an opcode+address) is issued,
because sometimes such splits are not supported by the memory.
^ permalink raw reply
* Re: [PATCH v3 20/21] ARM: dts: imx6ull-colibri: Add touchscreen used with Eval Board
From: Marcel Ziswiler @ 2019-08-09 15:54 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-imx@nxp.com, s.hauer@pengutronix.de, kernel@pengutronix.de,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190807082556.5013-21-philippe.schenker@toradex.com>
Hi Philippe
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This adds the common touchscreen that is used with Toradex's
> Eval Boards.
Is that really Eval Board specific?
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Removed f0710a, that is downstream only
> - Changed to generic node name
> - Better comment
>
> .../arm/boot/dts/imx6ull-colibri-eval-v3.dtsi | 24
> +++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> index d3c4809f140e..78e74bfeca1b 100644
> --- a/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx6ull-colibri-eval-v3.dtsi
> @@ -112,6 +112,21 @@
> &i2c1 {
> status = "okay";
>
> + /*
> + * Touchscreen is using SODIMM 28/30, also used for PWM<B>,
> PWM<C>,
> + * aka pwm2, pwm3. so if you enable touchscreen, disable the
> pwms
> + */
> + touchscreen@4a {
> + compatible = "atmel,maxtouch";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gpiotouch>;
> + reg = <0x4a>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <16 IRQ_TYPE_EDGE_FALLING>; /* SODIMM 28
> */
> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_HIGH>; /*
> SODIMM 30 */
> + status = "disabled";
> + };
> +
> /* M41T0M6 real time clock on carrier board */
> m41t0m6: rtc@68 {
> compatible = "st,m41t0";
> @@ -188,3 +203,12 @@
> sd-uhs-sdr104;
> status = "okay";
> };
> +
> +&iomuxc {
> + pinctrl_gpiotouch: touchgpios {
> + fsl,pins = <
> + MX6UL_PAD_NAND_DQS__GPIO4_IO16 0x74
> + MX6UL_PAD_ENET1_TX_EN__GPIO2_IO05 0x14
> + >;
> + };
> +};
I guess that could also be moved to the module's dtsi for any carrier board to potentially profit from.
Cheers
Marcel
^ permalink raw reply
* Re: [PATCH v3 21/21] ARM: dts: imx7-colibri: Add UHS support to eval board
From: Marcel Ziswiler @ 2019-08-09 15:56 UTC (permalink / raw)
To: Max Krummenacher, stefan@agner.ch, Philippe Schenker,
mark.rutland@arm.com, devicetree@vger.kernel.org,
michal.vokac@ysoft.com, shawnguo@kernel.org, festevam@gmail.com,
robh+dt@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, linux-kernel@vger.kernel.org,
linux-imx@nxp.com
In-Reply-To: <20190807082556.5013-22-philippe.schenker@toradex.com>
Hi Philippe
On Wed, 2019-08-07 at 08:26 +0000, Philippe Schenker wrote:
> This commit adds UHS capability to Toradex Eval Boards
How about any other carrier board?
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>
> ---
>
> Changes in v3:
> - New patch to make use of ARM: dts: imx7-colibri: fix 1.8V/UHS
> support
>
> Changes in v2: None
>
> arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> index 576dec9ff81c..90121fbe561f 100644
> --- a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi
> @@ -210,9 +210,16 @@
> };
>
> &usdhc1 {
> - keep-power-in-suspend;
> - wakeup-source;
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>;
> + pinctrl-1 = <&pinctrl_usdhc1_100mhz &pinctrl_cd_usdhc1>;
> + pinctrl-2 = <&pinctrl_usdhc1_200mhz &pinctrl_cd_usdhc1>;
> vmmc-supply = <®_3v3>;
> + vqmmc-supply = <®_LDO2>;
> + cd-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
> + disable-wp;
> + enable-sdio-wakeup;
> + keep-power-in-suspend;
> status = "okay";
> };
>
> --
> 2.22.0
Cheers
Marcel
^ permalink raw reply
* Re: [PATCH v2 0/9] Exynos Adaptive Supply Voltage support
From: Sylwester Nawrocki @ 2019-08-09 15:58 UTC (permalink / raw)
To: Viresh Kumar
Cc: Marek Szyprowski, krzk, robh+dt, vireshk, devicetree, kgene,
pankaj.dubey, linux-samsung-soc, linux-arm-kernel, linux-kernel,
linux-pm, b.zolnierkie
In-Reply-To: <20190725022343.p7lqalrh5svxvtu2@vireshk-i7>
Hi Viresh,
On 7/25/19 04:23, Viresh Kumar wrote:
> On 24-07-19, 15:10, Marek Szyprowski wrote:
>> On 2019-07-23 04:04, Viresh Kumar wrote:
>>> On 18-07-19, 16:30, Sylwester Nawrocki wrote:
>>>> This is second iteration of patch series adding ASV (Adaptive Supply
>>>> Voltage) support for Exynos SoCs. The first one can be found at:
>>>> https://lore.kernel.org/lkml/20190404171735.12815-1-s.nawrocki@samsung.com
>>>>
>>>> The main changes comparing to the first (RFC) version are:
>>>> - moving ASV data tables from DT to the driver,
>>>> - converting the chipid and the ASV drivers to use regmap,
>>>> - converting the ASV driver to proper platform driver.
>>>>
>>>> I tried the opp-supported-hw bitmask approach as in the Qualcomm CPUFreq
>>>> DT bindings but it resulted in too many OPPs and DT nodes, around 200
>>>> per CPU cluster. So the ASV OPP tables are now in the ASV driver, as in
>>>> downstream kernels.
>>>
>>> Hmm. Can you explain why do you have so many OPPs? How many
>>> frequencies do you actually support per cluster and what all varies
>>> per frequency based on hw ? How many hw version do u have ?
>>
>> For big cores there are 20 frequencies (2100MHz .. 200MHz). Each SoC
>> might belong to one of the 3 production 'sets' and each set contains 14
>> so called 'asv groups', which assign the certain voltage values for each
>> of those 20 frequencies (the lower asv group means lower voltage needed
>> for given frequency).
>
> There is another property which might be useful in this case:
> "opp-microvolt-<name>" and then you can use API
> dev_pm_opp_set_prop_name() to choose which voltage value to apply to
> all OPPs.
Thank you for your suggestions.
For some Exynos SoC variants the algorithm of selecting CPU voltage supply
is a bit more complex than just selecting a column in the frequency/voltage
matrix, i.e. selecting a set of voltage values for whole frequency range.
Frequency range could be divided into sub-ranges and to each such a sub-range
part of different column could be assigned, depending on data fused in
the CHIPID block registers.
We could create OPP node for each frequency and specify all needed voltages
as a list of "opp-microvolt-<name>" properties but apart from the fact that
it would have been quite many properties, e.g. 42 (3 tables * 14 columns),
only for some SoC types the dev_pm_opp_set_prop_name() approach could be
used. We would need to be able to set opp-microvolt-* property name
separately for each frequency (OPP).
Probably most future proof would be a DT binding where we could still
re-create those Exynos-specific ASV tables from DT. For example add named
opp-microvolt-* properties or something similar to hold rows of each ASV
table. But that conflicts with "operating-points-v2" binding, where
multiple OPP voltage values are described by just named properties and
multiple entries correspond to min/target/max.
opp_table0 {
compatible = "...", "operating-points-v2";
opp-shared;
opp-2100000000 {
opp-hz = /bits/ 64 <1800000000>;
opp-microvolt = <...>;
opp-microvolt-t1 = <1362500>, <1350000>, ....;
opp-microvolt-t2 = <1362500>, <1360000>, ....;
opp-microvolt-t3 = <1362500>, <1340000>, ....;
};
...
opp-200000000 {
opp-hz = /bits/ 64 <200000000>;
opp-microvolt = <...>;
opp-microvolt-t1 = <900000>, <900000>, ....;
opp-microvolt-t2 = <900000>, <900000>, ....;
opp-microvolt-t3 = <900000>, <900000>, ....;
};
};
I might be missing some information now on how those Exynos ASV tables
are used on other SoCs that would need to be supported.
There will be even more data to include when adding support for the Body
Bias voltage, for each CPU supply voltage we could possibly have
corresponding Body Bias voltage.
--
Thanks,
Sylwester
^ permalink raw reply
* Re: [PATCH v2 2/2] spi: npcm-fiu: add NPCM FIU controller driver
From: Tomer Maimon @ 2019-08-09 16:19 UTC (permalink / raw)
To: Boris Brezillon
Cc: Mark Brown, Rob Herring, Mark Rutland, Vignesh Raghavendra,
Boris Brezillon, Avi Fishman, Tali Perry, Patrick Venture,
Nancy Yuen, Benjamin Fair, linux-spi, devicetree,
OpenBMC Maillist, Linux Kernel Mailing List
In-Reply-To: <20190809175140.77747c8d@collabora.com>
[-- Attachment #1: Type: text/plain, Size: 3045 bytes --]
O.K.
Thanks a lot for the clarifications
I will add it in the next patch.
On Fri, 9 Aug 2019 at 18:51, Boris Brezillon <boris.brezillon@collabora.com>
wrote:
> On Fri, 9 Aug 2019 18:47:08 +0300
> Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> > On Fri, 9 Aug 2019 at 18:26, Boris Brezillon <
> boris.brezillon@collabora.com>
> > wrote:
> >
> > > On Fri, 9 Aug 2019 18:26:23 +0300
> > > Tomer Maimon <tmaimon77@gmail.com> wrote:
> > >
> > > > Hi Boris,
> > > >
> > > > Thanks a lot for your comment.
> > > >
> > > > On Thu, 8 Aug 2019 at 18:32, Boris Brezillon <
> > > boris.brezillon@collabora.com>
> > > > wrote:
> > > >
> > > > > On Thu, 8 Aug 2019 16:14:48 +0300
> > > > > Tomer Maimon <tmaimon77@gmail.com> wrote:
> > > > >
> > > > >
> > > > > > +
> > > > > > +static const struct spi_controller_mem_ops npcm_fiu_mem_ops = {
> > > > > > + .exec_op = npcm_fiu_exec_op,
> > > > >
> > > > > No npcm_supports_op()? That's suspicious, especially after looking
> at
> > > > > the npcm_fiu_exec_op() (and the functions called from there) where
> the
> > > > > requested ->buswidth seems to be completely ignored...
> > > > >
> > > > > Sorry but I do not fully understand it, do you mean a support for
> the
> > > > buswidth?
> > > > If yes it been done in the UMA functions as follow:
> > > >
> > > > uma_cfg |= ilog2(op->cmd.buswidth);
> > > > uma_cfg |= ilog2(op->addr.buswidth) <<
> > > > NPCM_FIU_UMA_CFG_ADBPCK_SHIFT;
> > > > uma_cfg |= ilog2(op->data.buswidth) <<
> > > > NPCM_FIU_UMA_CFG_WDBPCK_SHIFT;
> > > > uma_cfg |= op->addr.nbytes <<
> > > NPCM_FIU_UMA_CFG_ADDSIZ_SHIFT;
> > > > regmap_write(fiu->regmap, NPCM_FIU_UMA_ADDR,
> > > op->addr.val);
> > > >
> > >
> > > Hm, the default supports_op() implementation might be just fine for
> > > your use case. But there's one thing you still need to check: the
> > > number of addr cycles (or address size as you call it in this driver).
> > > Looks like your IP is limited to 4 address cycles, if I'm right, you
> > > should reject any operation that have op->addr.nbytes > 4. I also
> > >
> > Indeed our IP limited to 4 address cycle (bytes) do we have NOR Flash
> with
> > more than 32bit address?
>
> spi-mem is not only about spi-nor, it can be used for any kind of
> memory (NOR, NAND, SRAM, ...) or even to communicate with an FGPA, so
> yes, you have to take care of that.
>
> > I will add this limitation thanks!
> >
> > > wonder if there's a limitation on the data size you can have on a
> > > single transfer. If there's one you should implement ->adjust_op() too.
> > >
> > there is a limitation in a single transfer but I handle it in the
> > npcm_fiu_manualwrite
> > function.
> > Do you suggest to use ->adjust_op() instead?
>
> Yes, should be exposed through ->adjust_op() => the caller needs to
> know when a new operation (one containing an opcode+address) is issued,
> because sometimes such splits are not supported by the memory.
>
>
[-- Attachment #2: Type: text/html, Size: 4568 bytes --]
^ permalink raw reply
* Re: [PATCH v8 14/21] clk: tegra210: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-09 16:19 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree, rjw, viresh.kumar, linux-pm
In-Reply-To: <a21b7464-62c3-8461-04c2-a0e863bdde85@gmail.com>
On 8/9/19 6:56 AM, Dmitry Osipenko wrote:
> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>> This patch adds support for clk: tegra210: suspend-resume.
>>
>> All the CAR controller settings are lost on suspend when core
>> power goes off.
>>
>> This patch has implementation for saving and restoring all PLLs
>> and clocks context during system suspend and resume to have the
>> clocks back to same state for normal operation.
>>
>> Clock driver suspend and resume are registered as syscore_ops as clocks
>> restore need to happen before the other drivers resume to have all their
>> clocks back to the same state as before suspend.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/clk/tegra/clk-tegra210.c | 103 +++++++++++++++++++++++++++++++++++++--
>> drivers/clk/tegra/clk.c | 64 ++++++++++++++++++++++++
>> drivers/clk/tegra/clk.h | 3 ++
>> 3 files changed, 166 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>> index 998bf60b219a..8dd6f4f4debb 100644
>> --- a/drivers/clk/tegra/clk-tegra210.c
>> +++ b/drivers/clk/tegra/clk-tegra210.c
>> @@ -9,13 +9,13 @@
>> #include <linux/clkdev.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> +#include <linux/syscore_ops.h>
>> #include <linux/delay.h>
>> #include <linux/export.h>
>> #include <linux/mutex.h>
>> #include <linux/clk/tegra.h>
>> #include <dt-bindings/clock/tegra210-car.h>
>> #include <dt-bindings/reset/tegra210-car.h>
>> -#include <linux/iopoll.h>
>> #include <linux/sizes.h>
>> #include <soc/tegra/pmc.h>
>>
>> @@ -220,11 +220,15 @@
>> #define CLK_M_DIVISOR_SHIFT 2
>> #define CLK_M_DIVISOR_MASK 0x3
>>
>> +#define CLK_MASK_ARM 0x44
>> +#define MISC_CLK_ENB 0x48
>> +
>> #define RST_DFLL_DVCO 0x2f4
>> #define DVFS_DFLL_RESET_SHIFT 0
>>
>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>> +#define CPU_SOFTRST_CTRL 0x380
>>
>> #define LVL2_CLK_GATE_OVRA 0xf8
>> #define LVL2_CLK_GATE_OVRC 0x3a0
>> @@ -2825,6 +2829,7 @@ static int tegra210_enable_pllu(void)
>> struct tegra_clk_pll_freq_table *fentry;
>> struct tegra_clk_pll pllu;
>> u32 reg;
>> + int ret;
>>
>> for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>> if (fentry->input_rate == pll_ref_freq)
>> @@ -2853,9 +2858,14 @@ static int tegra210_enable_pllu(void)
>> reg |= PLL_ENABLE;
>> writel(reg, clk_base + PLLU_BASE);
>>
>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>> - reg & PLL_BASE_LOCK, 2, 1000);
>> - if (!(reg & PLL_BASE_LOCK)) {
>> + /*
>> + * During clocks resume, same PLLU init and enable sequence get
>> + * executed. So, readx_poll_timeout_atomic can't be used here as it
>> + * uses ktime_get() and timekeeping resume doesn't happen by that
>> + * time. So, using tegra210_wait_for_mask for PLL LOCK.
>> + */
>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>> + if (ret) {
>> pr_err("Timed out waiting for PLL_U to lock\n");
>> return -ETIMEDOUT;
>> }
>> @@ -3288,6 +3298,84 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>> }
>>
>> #ifdef CONFIG_PM_SLEEP
>> +/*
>> + * This array lists mask values for each peripheral clk bank
>> + * to mask out reserved bits during the clocks state restore
>> + * on SC7 resume to prevent accidental writes to these reserved
>> + * bits.
>> + */
>> +static u32 periph_clk_rsvd_mask[TEGRA210_CAR_BANK_COUNT] = {
> Should be more natural to have a "valid_mask" instead of "rsvd_mask".
>
> What's actually wrong with touching of the reserved bits? They must be NO-OP.. or the
> reserved bits are actually some kind of "secret" bits? If those bits have some use-case
> outside of Silicon HW (like FPGA simulation), then this doesn't matter for upstream and you
> have to keep the workaround locally in the downstream kernel or whatever.
Will rename as valid_mask.
some bits in these registers are undefined and is not good to write to
these bits as they can cause pslverr.
>
>> + 0x23282006,
>> + 0x782e0c18,
>> + 0x0c012c05,
>> + 0x003e7304,
>> + 0x86c04800,
>> + 0xc0199000,
>> + 0x03e03800,
>> +};
>> +
>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))
>> +#define car_writel(_val, _base, _off) \
>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>> +
>> +static u32 spare_reg_ctx, misc_clk_enb_ctx, clk_msk_arm_ctx;
>> +static u32 cpu_softrst_ctx[3];
>> +
>> +static int tegra210_clk_suspend(void)
>> +{
>> + unsigned int i;
>> +
>> + clk_save_context();
>> +
>> + /*
>> + * Save the bootloader configured clock registers SPARE_REG0,
>> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL.
>> + */
>> + spare_reg_ctx = readl_relaxed(clk_base + SPARE_REG0);
>> + misc_clk_enb_ctx = readl_relaxed(clk_base + MISC_CLK_ENB);
>> + clk_msk_arm_ctx = readl_relaxed(clk_base + CLK_MASK_ARM);
>> +
>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>> +
>> + tegra_clk_periph_suspend();
>> + return 0;
>> +}
>> +
>> +static void tegra210_clk_resume(void)
>> +{
>> + unsigned int i;
>> +
>> + tegra_clk_osc_resume(clk_base);
>> +
>> + /*
>> + * Restore the bootloader configured clock registers SPARE_REG0,
>> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL from saved context.
>> + */
>> + writel_relaxed(spare_reg_ctx, clk_base + SPARE_REG0);
>> + writel_relaxed(misc_clk_enb_ctx, clk_base + MISC_CLK_ENB);
>> + writel_relaxed(clk_msk_arm_ctx, clk_base + CLK_MASK_ARM);
>> +
>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>> +
>> + fence_udelay(5, clk_base);
>> +
>> + /* enable all the clocks before changing the clock sources */
>> + tegra_clk_periph_force_on(periph_clk_rsvd_mask);
> Why clocks need to be enabled before changing the sources?
To prevent glitchless frequency switch, Tegra clock programming
recommended sequence is to change MUX control or divisor or both with
the clocks running.
Actual state of clocks before suspend are restored later after all PLL's
and peripheral clocks are restored.
>
>> + /* wait for all writes to happen to have all the clocks enabled */
>> + wmb();
> fence_udelay() has exactly the same barrier at the very beginning of readl(), no need to
> duplicate it here.
>
>> + fence_udelay(2, clk_base);
>> +
>> + /* restore PLLs and all peripheral clock rates */
>> + tegra210_init_pllu();
> Why USB PLL need to be restored at first?
USB PLL restore is independent to all other clocks restore. So this can
be done either before clk_restore_context or even after.
>
>> + clk_restore_context();
>> +
>> + /* restore all peripheral clocks enable and reset state */
>> + tegra_clk_periph_resume();
>> +}
> [snip]
^ permalink raw reply
* Re: [PATCH 2/4] watchdog: Add i.MX7ULP watchdog support
From: Guenter Roeck @ 2019-08-09 16:19 UTC (permalink / raw)
To: Anson Huang
Cc: mark.rutland, devicetree, leonard.crestez, schnitzeltony,
linux-watchdog, otavio, festevam, s.hauer, jan.tuerk, linux,
linux-kernel, robh+dt, Linux-imx, kernel, u.kleine-koenig, wim,
shawnguo, linux-arm-kernel
In-Reply-To: <1565334842-28161-2-git-send-email-Anson.Huang@nxp.com>
On Fri, Aug 09, 2019 at 03:14:00PM +0800, Anson Huang wrote:
> The i.MX7ULP Watchdog Timer (WDOG) module is an independent timer
> that is available for system use.
> It provides a safety feature to ensure that software is executing
> as planned and that the CPU is not stuck in an infinite loop or
> executing unintended code. If the WDOG module is not serviced
> (refreshed) within a certain period, it resets the MCU.
>
> Add driver support for i.MX7ULP watchdog.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/watchdog/Kconfig | 13 +++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/imx7ulp_wdt.c | 221 +++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 235 insertions(+)
> create mode 100644 drivers/watchdog/imx7ulp_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 8188963..0884e53 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -740,6 +740,19 @@ config IMX_SC_WDT
> To compile this driver as a module, choose M here: the
> module will be called imx_sc_wdt.
>
> +config IMX7ULP_WDT
> + tristate "IMX7ULP Watchdog"
> + depends on ARCH_MXC || COMPILE_TEST
> + select WATCHDOG_CORE
> + help
> + This is the driver for the hardware watchdog on the Freescale
> + IMX7ULP and later processors. If you have one of these
> + processors and wish to have watchdog support enabled,
> + say Y, otherwise say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called imx7ulp_wdt.
> +
> config UX500_WATCHDOG
> tristate "ST-Ericsson Ux500 watchdog"
> depends on MFD_DB8500_PRCMU
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 7caa920..7d32537 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_TS4800_WATCHDOG) += ts4800_wdt.o
> obj-$(CONFIG_TS72XX_WATCHDOG) += ts72xx_wdt.o
> obj-$(CONFIG_IMX2_WDT) += imx2_wdt.o
> obj-$(CONFIG_IMX_SC_WDT) += imx_sc_wdt.o
> +obj-$(CONFIG_IMX7ULP_WDT) += imx7ulp_wdt.o
> obj-$(CONFIG_UX500_WATCHDOG) += ux500_wdt.o
> obj-$(CONFIG_RETU_WATCHDOG) += retu_wdt.o
> obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> new file mode 100644
> index 0000000..8d56023
> --- /dev/null
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reboot.h>
> +#include <linux/watchdog.h>
> +
> +#define WDOG_CS 0x0
> +#define WDOG_CS_CMD32EN (1 << 13)
> +#define WDOG_CS_ULK (1 << 11)
> +#define WDOG_CS_RCS (1 << 10)
> +#define WDOG_CS_EN (1 << 7)
> +#define WDOG_CS_UPDATE (1 << 5)
> +
Please use BIT() where appropriate.
> +#define WDOG_CNT 0x4
> +#define WDOG_TOVAL 0x8
> +
> +#define REFRESH_SEQ0 0xA602
> +#define REFRESH_SEQ1 0xB480
> +#define REFRESH ((REFRESH_SEQ1 << 16) | (REFRESH_SEQ0))
The inner ( ) are unnecessary. While I would accept it for readability
for the first part, (REFRESH_SEQ0) really doesn't add value.
> +
> +#define UNLOCK_SEQ0 0xC520
> +#define UNLOCK_SEQ1 0xD928
> +#define UNLOCK ((UNLOCK_SEQ1 << 16) | (UNLOCK_SEQ0))
Same as above.
> +
> +#define DEFAULT_TIMEOUT 60
> +#define MAX_TIMEOUT 128
tabs after _TIMEOUT, please
> +
> +static bool nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, bool, 0000);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
> + __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
> +
> +struct imx7ulp_wdt_device {
> + struct notifier_block restart_handler;
> + struct watchdog_device wdd;
> + void __iomem *base;
> + int rate;
> +};
> +
> +static inline void imx7ulp_wdt_enable(void __iomem *base, bool enable)
> +{
> + u32 val = readl(base + WDOG_CS);
> +
> + writel(UNLOCK, base + WDOG_CNT);
> + if (enable)
> + writel(val | WDOG_CS_EN, base + WDOG_CS);
> + else
> + writel(val & ~WDOG_CS_EN, base + WDOG_CS);
> +}
> +
> +static inline bool imx7ulp_wdt_is_enabled(void __iomem *base)
> +{
> + u32 val = readl(base + WDOG_CS);
> +
> + return val & WDOG_CS_EN;
> +}
> +
> +static int imx7ulp_wdt_ping(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + writel(REFRESH, wdt->base + WDOG_CNT);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_start(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + imx7ulp_wdt_enable(wdt->base, true);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_stop(struct watchdog_device *wdog)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> +
> + imx7ulp_wdt_enable(wdt->base, false);
> +
> + return 0;
> +}
> +
> +static int imx7ulp_wdt_set_timeout(struct watchdog_device *wdog,
> + unsigned int timeout)
> +{
> + struct imx7ulp_wdt_device *wdt = watchdog_get_drvdata(wdog);
> + u32 val = wdt->rate * timeout;
> +
> + writel(UNLOCK, wdt->base + WDOG_CNT);
> + writel(val, wdt->base + WDOG_TOVAL);
> +
> + wdog->timeout = timeout;
> +
> + return 0;
> +}
> +
> +static const struct watchdog_ops imx7ulp_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = imx7ulp_wdt_start,
> + .stop = imx7ulp_wdt_stop,
> + .ping = imx7ulp_wdt_ping,
> + .set_timeout = imx7ulp_wdt_set_timeout,
> +};
> +
> +static const struct watchdog_info imx7ulp_wdt_info = {
> + .identity = "i.MX7ULP watchdog timer",
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING |
> + WDIOF_MAGICCLOSE,
> +};
> +
> +static inline void imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
> +{
> + u32 val;
> +
> + /* unlock the wdog for reconfiguration */
> + writel_relaxed(UNLOCK_SEQ0, base + WDOG_CNT);
> + writel_relaxed(UNLOCK_SEQ1, base + WDOG_CNT);
> +
> + /* set an initial timeout value in TOVAL */
> + writel(timeout, base + WDOG_TOVAL);
> + /* enable 32bit command sequence and reconfigure */
> + val = (1 << 13) | (1 << 8) | (1 << 5);
Please use BIT()
> + writel(val, base + WDOG_CS);
> +}
> +
> +static int imx7ulp_wdt_probe(struct platform_device *pdev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt;
> + struct device *dev = &pdev->dev;
> + struct watchdog_device *wdog;
> + int ret;
> +
> + imx7ulp_wdt = devm_kzalloc(&pdev->dev,
> + sizeof(*imx7ulp_wdt), GFP_KERNEL);
> + if (!imx7ulp_wdt)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, imx7ulp_wdt);
> +
> + imx7ulp_wdt->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(imx7ulp_wdt->base))
> + return PTR_ERR(imx7ulp_wdt->base);
> +
> + imx7ulp_wdt->rate = 1000;
> + wdog = &imx7ulp_wdt->wdd;
> + wdog->info = &imx7ulp_wdt_info;
> + wdog->ops = &imx7ulp_wdt_ops;
> + wdog->min_timeout = 1;
> + wdog->max_timeout = MAX_TIMEOUT;
> + wdog->parent = dev;
> + wdog->timeout = DEFAULT_TIMEOUT;
> +
> + watchdog_init_timeout(wdog, 0, dev);
> + watchdog_stop_on_reboot(wdog);
> + watchdog_stop_on_unregister(wdog);
> + watchdog_set_drvdata(wdog, imx7ulp_wdt);
> + imx7ulp_wdt_init(imx7ulp_wdt->base, wdog->timeout * imx7ulp_wdt->rate);
> +
> + ret = devm_watchdog_register_device(dev, wdog);
> + if (ret)
> + dev_err(dev, "Failed to register watchdog device\n");
An error message is already displayed by the watchdog core.
> +
> + return ret;
> +}
> +
> +static int __maybe_unused imx7ulp_wdt_suspend(struct device *dev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> +
> + if (watchdog_active(&imx7ulp_wdt->wdd))
> + imx7ulp_wdt_stop(&imx7ulp_wdt->wdd);
> +
> + return 0;
> +}
> +
> +static int __maybe_unused imx7ulp_wdt_resume(struct device *dev)
> +{
> + struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
> + u32 timeout = imx7ulp_wdt->wdd.timeout * imx7ulp_wdt->rate;
> +
> + if (imx7ulp_wdt_is_enabled(imx7ulp_wdt->base))
> + imx7ulp_wdt_init(imx7ulp_wdt->base, timeout);
> +
If I understand correctly, imx7ulp_wdt_is_enabled() returns true
if the watchdog is running. Since it was stopped on suspend, that
means that it was started in BIOS/rommon during resume.
With that, the above translates to "If the watchdog was started by
BIOS/rommon, re-initialize it. Otherwise do nothing". This doesn't
really make much sense to me. What if the watchdog was reprogrammed
by the BIOS/rommon, but not started ? In other words, why not call
imx7ulp_wdt_init() unconditionally ?
Also, if it is possible that the watchdog is started by BIOS/rommon,
why not keep it enabled and tell the watchdog core about it in
the probe function ?
> + if (watchdog_active(&imx7ulp_wdt->wdd))
> + imx7ulp_wdt_start(&imx7ulp_wdt->wdd);
> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(imx7ulp_wdt_pm_ops, imx7ulp_wdt_suspend,
> + imx7ulp_wdt_resume);
> +
> +static const struct of_device_id imx7ulp_wdt_dt_ids[] = {
> + { .compatible = "fsl,imx7ulp-wdt", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx7ulp_wdt_dt_ids);
> +
> +static struct platform_driver imx7ulp_wdt_driver = {
> + .probe = imx7ulp_wdt_probe,
> + .driver = {
> + .name = "imx7ulp-wdt",
> + .pm = &imx7ulp_wdt_pm_ops,
> + .of_match_table = imx7ulp_wdt_dt_ids,
> + },
> +};
> +module_platform_driver(imx7ulp_wdt_driver);
> +
> +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> +MODULE_DESCRIPTION("Freescale i.MX7ULP watchdog driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v8 19/21] soc/tegra: pmc: Configure deep sleep control settings
From: Sowjanya Komatineni @ 2019-08-09 16:23 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree, rjw, viresh.kumar, linux-pm
In-Reply-To: <57ed54cd-bf57-cab1-eb63-8548761640de@gmail.com>
On 8/9/19 6:23 AM, Dmitry Osipenko wrote:
> 09.08.2019 2:46, Sowjanya Komatineni пишет:
>> Tegra210 and prior Tegra chips have deep sleep entry and wakeup related
>> timings which are platform specific that should be configured before
>> entering into deep sleep.
>>
>> Below are the timing specific configurations for deep sleep entry and
>> wakeup.
>> - Core rail power-on stabilization timer
>> - OSC clock stabilization timer after SOC rail power is stabilized.
>> - Core power off time is the minimum wake delay to keep the system
>> in deep sleep state irrespective of any quick wake event.
>>
>> These values depends on the discharge time of regulators and turn OFF
>> time of the PMIC to allow the complete system to finish entering into
>> deep sleep state.
>>
>> These values vary based on the platform design and are specified
>> through the device tree.
>>
>> This patch has implementation to configure these timings which are must
>> to have for proper deep sleep and wakeup operations.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/soc/tegra/pmc.c | 13 ++++++++++++-
>> 1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index e013ada7e4e9..9a78d8417367 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -88,6 +88,8 @@
>>
>> #define PMC_CPUPWRGOOD_TIMER 0xc8
>> #define PMC_CPUPWROFF_TIMER 0xcc
>> +#define PMC_COREPWRGOOD_TIMER 0x3c
>> +#define PMC_COREPWROFF_TIMER 0xe0
>>
>> #define PMC_PWR_DET_VALUE 0xe4
>>
>> @@ -2277,7 +2279,7 @@ static const struct tegra_pmc_regs tegra20_pmc_regs = {
>>
>> static void tegra20_pmc_init(struct tegra_pmc *pmc)
>> {
>> - u32 value;
>> + u32 value, osc, pmu, off;
>>
>> /* Always enable CPU power request */
>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>> @@ -2303,6 +2305,15 @@ static void tegra20_pmc_init(struct tegra_pmc *pmc)
>> value = tegra_pmc_readl(pmc, PMC_CNTRL);
>> value |= PMC_CNTRL_SYSCLK_OE;
>> tegra_pmc_writel(pmc, value, PMC_CNTRL);
>> +
>> + osc = DIV_ROUND_UP(pmc->core_osc_time * 8192, 1000000);
>> + pmu = DIV_ROUND_UP(pmc->core_pmu_time * 32768, 1000000);
>> + off = DIV_ROUND_UP(pmc->core_off_time * 32768, 1000000);
>> + if (osc && pmu)
>> + tegra_pmc_writel(pmc, ((osc << 8) & 0xff00) | (pmu & 0xff),
>> + PMC_COREPWRGOOD_TIMER);
>> + if (off)
>> + tegra_pmc_writel(pmc, off, PMC_COREPWROFF_TIMER);
> The osc/pmu/off values are undefined if they are not defined in device-tree. I suppose this
> need to be corrected in tegra_pmc_parse_dt() if the values really matter even if LP0 suspend
> isn't supported in device-tree.
>
> And I'm also not sure what's wrong with setting 0 for the timers.
>
These settings are for SC7 only and will not have any impact in normal
state.
>> }
>>
>> static void tegra20_pmc_setup_irq_polarity(struct tegra_pmc *pmc,
>>
^ permalink raw reply
* [PATCH v2 0/3] drm: bridge: Add NWL MIPI DSI host controller support
From: Guido Günther @ 2019-08-09 16:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Lee Jones, Guido Günther,
dri-devel, devicetree, linux-arm-kernel, linux-kernel,
Robert Chiras, Sam Ravnborg
This adds initial support for the NWL MIPI DSI Host controller found on i.MX8
SoCs.
It adds support for the i.MX8MQ but the same IP core can also be found on e.g.
i.MX8QXP. I added the necessary hooks to support other imx8 variants but since
I only have imx8mq boards to test I omitted the platform data for other SoCs.
The code is based on NXPs BSP so I added Robert Chiras as
Co-authored-by. Robert, if this looks sane could you add your
Signed-off-by:?
The most notable changes over the BSP driver are
- Calculate HS mode timing from phy_configure_opts_mipi_dphy
- Perform all clock setup via DT
- Merge nwl-imx and nwl drivers
- Add B0 silion revision quirk
- Become a bridge driver to hook into mxsfb (from what I read[0] DCSS, which
also can drive the nwl on the imx8mq will likely not become part of
imx-display-subsystem so it makes sense to make it drive a bridge for dsi as
well).
- Use panel_bridge to attach the panel
This has been tested on a Librem 5 devkit using mxsfb with Robert's patches[1]
and the rocktech-jh057n00900 panel driver on next-20190807. The DCSS can later
on also act as input source too.
Changes from v1:
- Per review comments by Sam Ravnborg
https://lists.freedesktop.org/archives/dri-devel/2019-July/228130.html
- Change binding docs to YAML
- build: Don't always visit imx-nwl/
- build: Add header-test-y
- Sort headers according to DRM convention
- Use drm_display_mode instead of videmode
- Per review comments by Fabio Estevam
https://lists.freedesktop.org/archives/dri-devel/2019-July/228299.html
- Don't restrict build to ARCH_MXC
- Drop unused includes
- Drop unreachable code in imx_nwl_dsi_bridge_mode_fixup()
- Drop remaining calls of dev_err() and use DRM_DEV_ERR()
consistently.
- Use devm_platform_ioremap_resource()
- Drop devm_free_irq() in probe() error path
- Use single line comments where sufficient
- Use <linux/time64.h> instead of defining USEC_PER_SEC
- Make input source select imx8 specific
- Drop <asm/unaligned.h> inclusion (after removal of get_unaligned_le32)
- Drop all EXPORT_SYMBOL_GPL() for functions used in the same module
but different source files.
- Drop nwl_dsi_enable_{rx,tx}_clock() by invoking clk_prepare_enable()
directly
- Remove pointless comment
- Laurent Pinchart
https://lists.freedesktop.org/archives/dri-devel/2019-July/228313.html
https://lists.freedesktop.org/archives/dri-devel/2019-July/228308.html
- Drop (on iMX8MQ) unused csr regmap
- Use NWL_MAX_PLATFORM_CLOCKS everywhere
- Drop get_unaligned_le32() usage
- remove duplicate 'for the' in binding docs
- Don't include unused <linux/clk-provider.h>
- Don't include unused <linux/component.h>
- Drop dpms_mode for tracking state, trust the drm layer on that
- Use pm_runtime_put() instead of pm_runtime_put_sync()
- Don't overwrite encoder type
- Make imx_nwl_platform_data const
- Use the reset controller API instead of open coding that platform specific
part
- Use <linux/bitfield.h> intead of making up our own defines
- name mipi_dsi_transfer less generic: nwl_dsi_transfer
- ensure clean in .remove by calling mipi_dsi_host_unregister.
- prefix constants by NWL_DSI_
- properly format transfer_direction enum
- simplify platform clock handling
- Don't modify state in mode_fixup() and use mode_set() instead
- Drop bridge detach(), already handle by nwl_dsi_host_detach()
- Drop USE_*_QUIRK() macros
- Drop (for now) unused clock defnitions. 'pixel' and 'bypass' clock will be
used for i.MX8 SoCs but since they're unused atm drop the definitions - but
keep the logic to enable/disable several clocks in place since we know we'll
need it in the future.
Changes from v0:
- Add quirk for IMQ8MQ silicon B0 revision to not mess with the
system reset controller on power down since enable() won't work
otherwise.
- Drop devm_free_irq() handled by the device driver core
- Disable tx esc clock after the phy power down to unbreak
disable/enable (unblank/blank)
- Add ports to dt binding docs
- Select GENERIC_PHY_MIPI_DPHY instead of GENERIC_PHY for
phy_mipi_dphy_get_default_config
- Select DRM_MIPI_DSI
- Include drm_print.h to fix build on next-20190408
- Drop some debugging messages
- Newline terminate all DRM_ printouts
- Turn component driver into a drm bridge
[0]: https://lists.freedesktop.org/archives/dri-devel/2019-May/219484.html
[1]: https://patchwork.freedesktop.org/series/62822/
Guido Günther (3):
arm64: imx8mq: add imx8mq iomux-gpr field defines
dt-bindings: display/bridge: Add binding for NWL mipi dsi host
controller
drm/bridge: Add NWL MIPI DSI host controller support
.../bindings/display/bridge/nwl-dsi.yaml | 155 ++++
drivers/gpu/drm/bridge/Kconfig | 2 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/nwl-dsi/Kconfig | 15 +
drivers/gpu/drm/bridge/nwl-dsi/Makefile | 4 +
drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c | 484 ++++++++++++
drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h | 66 ++
drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c | 700 ++++++++++++++++++
drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h | 112 +++
include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h | 62 ++
10 files changed, 1601 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Kconfig
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Makefile
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
create mode 100644 include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v2 1/3] arm64: imx8mq: add imx8mq iomux-gpr field defines
From: Guido Günther @ 2019-08-09 16:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Lee Jones, Guido Günther,
dri-devel, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1565367567.git.agx@sigxcpu.org>
This adds all the gpr registers and the define needed for selecting
the input source in the imx-nwl drm bridge.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h | 62 ++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h
diff --git a/include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h b/include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h
new file mode 100644
index 000000000000..62e85ffacfad
--- /dev/null
+++ b/include/linux/mfd/syscon/imx8mq-iomuxc-gpr.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2017 NXP
+ * 2019 Purism SPC
+ */
+
+#ifndef __LINUX_IMX8MQ_IOMUXC_GPR_H
+#define __LINUX_IMX8MQ_IOMUXC_GPR_H
+
+#define IOMUXC_GPR0 0x00
+#define IOMUXC_GPR1 0x04
+#define IOMUXC_GPR2 0x08
+#define IOMUXC_GPR3 0x0c
+#define IOMUXC_GPR4 0x10
+#define IOMUXC_GPR5 0x14
+#define IOMUXC_GPR6 0x18
+#define IOMUXC_GPR7 0x1c
+#define IOMUXC_GPR8 0x20
+#define IOMUXC_GPR9 0x24
+#define IOMUXC_GPR10 0x28
+#define IOMUXC_GPR11 0x2c
+#define IOMUXC_GPR12 0x30
+#define IOMUXC_GPR13 0x34
+#define IOMUXC_GPR14 0x38
+#define IOMUXC_GPR15 0x3c
+#define IOMUXC_GPR16 0x40
+#define IOMUXC_GPR17 0x44
+#define IOMUXC_GPR18 0x48
+#define IOMUXC_GPR19 0x4c
+#define IOMUXC_GPR20 0x50
+#define IOMUXC_GPR21 0x54
+#define IOMUXC_GPR22 0x58
+#define IOMUXC_GPR23 0x5c
+#define IOMUXC_GPR24 0x60
+#define IOMUXC_GPR25 0x64
+#define IOMUXC_GPR26 0x68
+#define IOMUXC_GPR27 0x6c
+#define IOMUXC_GPR28 0x70
+#define IOMUXC_GPR29 0x74
+#define IOMUXC_GPR30 0x78
+#define IOMUXC_GPR31 0x7c
+#define IOMUXC_GPR32 0x80
+#define IOMUXC_GPR33 0x84
+#define IOMUXC_GPR34 0x88
+#define IOMUXC_GPR35 0x8c
+#define IOMUXC_GPR36 0x90
+#define IOMUXC_GPR37 0x94
+#define IOMUXC_GPR38 0x98
+#define IOMUXC_GPR39 0x9c
+#define IOMUXC_GPR40 0xa0
+#define IOMUXC_GPR41 0xa4
+#define IOMUXC_GPR42 0xa8
+#define IOMUXC_GPR43 0xac
+#define IOMUXC_GPR44 0xb0
+#define IOMUXC_GPR45 0xb4
+#define IOMUXC_GPR46 0xb8
+#define IOMUXC_GPR47 0xbc
+
+/* i.MX8Mq iomux gpr register field defines */
+#define IMX8MQ_GPR13_MIPI_MUX_SEL BIT(2)
+
+#endif /* __LINUX_IMX8MQ_IOMUXC_GPR_H */
--
2.20.1
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: display/bridge: Add binding for NWL mipi dsi host controller
From: Guido Günther @ 2019-08-09 16:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Lee Jones, Guido Günther,
dri-devel, devicetree, linux-arm-kernel, linux-kernel,
Robert Chiras, Sam Ravnborg
In-Reply-To: <cover.1565367567.git.agx@sigxcpu.org>
The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
.../bindings/display/bridge/nwl-dsi.yaml | 155 ++++++++++++++++++
1 file changed, 155 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
new file mode 100644
index 000000000000..5ed8bc4a4d18
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/imx-nwl-dsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Northwest Logic MIPI-DSI on imx SoCs
+
+maintainers:
+ - Guido Gúnther <agx@sigxcpu.org>
+ - Robert Chiras <robert.chiras@nxp.com>
+
+description: |
+ NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
+ the SOCs NWL MIPI-DSI host controller.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - const: fsl,imx8mq-nwl-dsi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: DSI core clock
+ - description: RX_ESC clock (used in escape mode)
+ - description: TX_ESC clock (used in escape mode)
+ - description: PHY_REF clock
+
+ clock-names:
+ items:
+ - const: core
+ - const: rx_esc
+ - const: tx_esc
+ - const: phy_ref
+
+ phys:
+ maxItems: 1
+ description:
+ A phandle to the phy module representing the DPHY
+
+ phy-names:
+ items:
+ - const: dphy
+
+ power-domains:
+ maxItems: 1
+ description:
+ A phandle to the power domain
+
+ resets:
+ maxItems: 4
+ description:
+ A phandle to the reset controller
+
+ reset-names:
+ items:
+ - const: byte
+ - const: dpi
+ - const: esc
+ - const: pclk
+
+ mux-sel:
+ maxItems: 1
+ description:
+ A phandle to the MUX register set
+
+ port:
+ type: object
+ description:
+ A input put or output port node.
+
+ ports:
+ type: object
+ description:
+ A node containing DSI input & output port nodes with endpoint
+ definitions as documented in
+ Documentation/devicetree/bindings/graph.txt.
+
+patternProperties:
+ "^panel@[0-9]+$": true
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8mq-nwl-dsi
+ then:
+ required:
+ - resets
+ - reset-names
+ - mux-sel
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - phys
+ - phy-names
+
+examples:
+ - |
+
+ mipi_dsi: mipi_dsi@30a00000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx8mq-nwl-dsi";
+ reg = <0x30A00000 0x300>;
+ clocks = <&clk 163>, <&clk 244>, <&clk 245>, <&clk 164>;
+ clock-names = "core", "rx_esc", "tx_esc", "phy_ref";
+ interrupts = <0 34 4>;
+ power-domains = <&pgc_mipi>;
+ resets = <&src 0>, <&src 1>, <&src 2>, <&src 3>;
+ reset-names = "byte", "dpi", "esc", "pclk";
+ mux-sel = <&iomuxc_gpr>;
+ phys = <&dphy>;
+ phy-names = "dphy";
+
+ panel@0 {
+ compatible = "...";
+ port@0 {
+ panel_in: endpoint {
+ remote-endpoint = <&mipi_dsi_out>;
+ };
+ };
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ mipi_dsi_in: endpoint {
+ remote-endpoint = <&lcdif_mipi_dsi>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ mipi_dsi_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+ };
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* [PATCH v2 3/3] drm/bridge: Add NWL MIPI DSI host controller support
From: Guido Günther @ 2019-08-09 16:24 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Lee Jones, Guido Günther,
dri-devel, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1565367567.git.agx@sigxcpu.org>
This adds initial support for the NWL MIPI DSI Host controller found on
i.MX8 SoCs.
It adds support for the i.MX8MQ but the same IP can be found on
e.g. the i.MX8QXP.
It has been tested on the Librem 5 devkit using mxsfb.
Signed-off-by: Guido Günther <agx@sigxcpu.org>
Co-developed-by: Robert Chiras <robert.chiras@nxp.com>
---
drivers/gpu/drm/bridge/Kconfig | 2 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/nwl-dsi/Kconfig | 15 +
drivers/gpu/drm/bridge/nwl-dsi/Makefile | 4 +
drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c | 484 ++++++++++++++++
drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h | 66 +++
drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c | 700 +++++++++++++++++++++++
drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h | 112 ++++
8 files changed, 1384 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Kconfig
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/Makefile
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
create mode 100644 drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 1cc9f502c1f2..7980b5c2156f 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -154,6 +154,8 @@ source "drivers/gpu/drm/bridge/analogix/Kconfig"
source "drivers/gpu/drm/bridge/adv7511/Kconfig"
+source "drivers/gpu/drm/bridge/nwl-dsi/Kconfig"
+
source "drivers/gpu/drm/bridge/synopsys/Kconfig"
endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 4934fcf5a6f8..d9f6c0f77592 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -16,4 +16,5 @@ obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
+obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-dsi/
obj-y += synopsys/
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Kconfig b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig
new file mode 100644
index 000000000000..27ec86c05401
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/Kconfig
@@ -0,0 +1,15 @@
+config DRM_NWL_MIPI_DSI
+ tristate "Support for Northwest Logic MIPI DSI Host controller"
+ depends on DRM
+ depends on COMMON_CLK
+ depends on OF && HAS_IOMEM
+ select DRM_KMS_HELPER
+ select DRM_MIPI_DSI
+ select DRM_PANEL_BRIDGE
+ select GENERIC_PHY_MIPI_DPHY
+ select MFD_SYSCON
+ select REGMAP_MMIO
+ help
+ This enables the Northwest Logic MIPI DSI Host controller as
+ for example found on NXP's i.MX8 Processors.
+
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/Makefile b/drivers/gpu/drm/bridge/nwl-dsi/Makefile
new file mode 100644
index 000000000000..804baf2f1916
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+nwl-mipi-dsi-y := nwl-drv.o nwl-dsi.o
+obj-$(CONFIG_DRM_NWL_MIPI_DSI) += nwl-mipi-dsi.o
+header-test-y += nwl-drv.h nwl-dsi.h
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
new file mode 100644
index 000000000000..0bd3a4184885
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.c
@@ -0,0 +1,484 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * i.MX8 NWL MIPI DSI host driver
+ *
+ * Copyright (C) 2017 NXP
+ * Copyright (C) 2019 Purism SPC
+ */
+
+#include <linux/clk.h>
+#include <linux/irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx8mq-iomuxc-gpr.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+#include <linux/sys_soc.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_print.h>
+#include <drm/drm_probe_helper.h>
+
+#include "nwl-drv.h"
+#include "nwl-dsi.h"
+
+#define DRV_NAME "nwl-dsi"
+
+/* Possible platform specific clocks */
+#define NWL_DSI_CLK_CORE "core"
+
+enum nwl_dsi_ext_regs {
+ NWL_DSI_IMX_REG_GPR = BIT(1),
+};
+
+static const struct regmap_config nwl_dsi_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = NWL_DSI_IRQ_MASK2,
+ .name = DRV_NAME,
+};
+
+struct nwl_dsi_platform_data {
+ int (*poweron)(struct nwl_dsi *dsi);
+ int (*poweroff)(struct nwl_dsi *dsi);
+ void (*probe)(struct nwl_dsi *dsi);
+ void (*select_input)(struct nwl_dsi *dsi);
+ u32 ext_regs; /* required external registers */
+ struct nwl_dsi_plat_clk_config clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS];
+};
+
+static inline struct nwl_dsi *bridge_to_dsi(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct nwl_dsi, bridge);
+}
+
+static int nwl_dsi_set_platform_clocks(struct nwl_dsi *dsi, bool enable)
+{
+ struct device *dev = dsi->dev;
+ const char *id;
+ struct clk *clk;
+ size_t i;
+ unsigned long rate;
+ int ret, result = 0;
+
+ DRM_DEV_DEBUG_DRIVER(dev, "%s platform clocks\n",
+ enable ? "enabling" : "disabling");
+ for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
+ if (!dsi->clk_config[i].present)
+ continue;
+ id = dsi->clk_config[i].id;
+ clk = dsi->clk_config[i].clk;
+
+ if (enable) {
+ ret = clk_prepare_enable(clk);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev,
+ "Failed to enable %s clk: %d\n",
+ id, ret);
+ result = result ?: ret;
+ }
+ rate = clk_get_rate(clk);
+ DRM_DEV_DEBUG_DRIVER(dev, "Enabled %s clk @%lu Hz\n",
+ id, rate);
+ } else {
+ clk_disable_unprepare(clk);
+ DRM_DEV_DEBUG_DRIVER(dev, "Disabled %s clk\n", id);
+ }
+ }
+
+ return result;
+}
+
+static int nwl_dsi_plat_enable(struct nwl_dsi *dsi)
+{
+ struct device *dev = dsi->dev;
+ int ret;
+
+ ret = nwl_dsi_set_platform_clocks(dsi, true);
+ if (ret < 0)
+ return ret;
+
+ ret = dsi->pdata->poweron(dsi);
+ if (ret < 0)
+ DRM_DEV_ERROR(dev, "Failed to power on DSI: %d\n", ret);
+ return ret;
+}
+
+static void nwl_dsi_plat_disable(struct nwl_dsi *dsi)
+{
+ dsi->pdata->poweroff(dsi);
+ nwl_dsi_set_platform_clocks(dsi, false);
+}
+
+static void nwl_dsi_bridge_disable(struct drm_bridge *bridge)
+{
+ struct nwl_dsi *dsi = bridge_to_dsi(bridge);
+
+ nwl_dsi_disable(dsi);
+ nwl_dsi_plat_disable(dsi);
+ pm_runtime_put(dsi->dev);
+}
+
+static int nwl_dsi_get_dphy_params(struct nwl_dsi *dsi,
+ const struct drm_display_mode *mode,
+ union phy_configure_opts *phy_opts)
+{
+ unsigned long rate;
+ int ret;
+
+ if (dsi->lanes < 1 || dsi->lanes > 4)
+ return -EINVAL;
+
+ /*
+ * So far the DPHY spec minimal timings work for both mixel
+ * dphy and nwl dsi host
+ */
+ ret = phy_mipi_dphy_get_default_config(
+ mode->crtc_clock * 1000,
+ mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes,
+ &phy_opts->mipi_dphy);
+ if (ret < 0)
+ return ret;
+
+ rate = clk_get_rate(dsi->tx_esc_clk);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "LP clk is @%lu Hz\n", rate);
+ phy_opts->mipi_dphy.lp_clk_rate = rate;
+
+ return 0;
+}
+
+static bool nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ struct drm_display_mode *adjusted_mode)
+{
+ /* At least LCDIF + NWL needs active high sync */
+ adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
+ adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
+
+ return true;
+}
+
+static enum drm_mode_status
+nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode)
+{
+ struct nwl_dsi *dsi = bridge_to_dsi(bridge);
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ if (mode->clock * bpp > 15000000)
+ return MODE_CLOCK_HIGH;
+
+ if (mode->clock * bpp < 80000)
+ return MODE_CLOCK_LOW;
+
+ return MODE_OK;
+}
+
+static void
+nwl_dsi_bridge_mode_set(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
+{
+ struct nwl_dsi *dsi = bridge_to_dsi(bridge);
+ struct device *dev = dsi->dev;
+ union phy_configure_opts new_cfg;
+ unsigned long phy_ref_rate;
+ int ret;
+
+ ret = nwl_dsi_get_dphy_params(dsi, adjusted_mode, &new_cfg);
+ if (ret < 0)
+ return;
+
+ /*
+ * If hs clock is unchanged, we're all good - all parameters are
+ * derived from it atm.
+ */
+ if (new_cfg.mipi_dphy.hs_clk_rate == dsi->phy_cfg.mipi_dphy.hs_clk_rate)
+ return;
+
+ phy_ref_rate = clk_get_rate(dsi->phy_ref_clk);
+ DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n", phy_ref_rate);
+ /* Save the new desired phy config */
+ memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg));
+
+ memcpy(&dsi->mode, adjusted_mode, sizeof(dsi->mode));
+ drm_mode_debug_printmodeline(adjusted_mode);
+}
+
+static void nwl_dsi_bridge_pre_enable(struct drm_bridge *bridge)
+{
+ struct nwl_dsi *dsi = bridge_to_dsi(bridge);
+
+ dsi->pdata->select_input(dsi);
+ pm_runtime_get_sync(dsi->dev);
+ nwl_dsi_plat_enable(dsi);
+ nwl_dsi_enable(dsi);
+}
+
+static int nwl_dsi_bridge_attach(struct drm_bridge *bridge)
+{
+ struct nwl_dsi *dsi = bridge->driver_private;
+
+ return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge);
+}
+
+static const struct drm_bridge_funcs nwl_dsi_bridge_funcs = {
+ .pre_enable = nwl_dsi_bridge_pre_enable,
+ .disable = nwl_dsi_bridge_disable,
+ .mode_fixup = nwl_dsi_bridge_mode_fixup,
+ .mode_set = nwl_dsi_bridge_mode_set,
+ .mode_valid = nwl_dsi_bridge_mode_valid,
+ .attach = nwl_dsi_bridge_attach,
+};
+
+static int nwl_dsi_parse_dt(struct nwl_dsi *dsi)
+{
+ struct device_node *np = dsi->dev->of_node;
+ struct platform_device *pdev = to_platform_device(dsi->dev);
+ struct clk *clk;
+ const char *clk_id;
+ void __iomem *base;
+ int i, ret;
+
+ dsi->phy = devm_phy_get(dsi->dev, "dphy");
+ if (IS_ERR(dsi->phy)) {
+ ret = PTR_ERR(dsi->phy);
+ DRM_DEV_ERROR(dsi->dev, "Could not get PHY: %d\n", ret);
+ return ret;
+ }
+
+ /* Platform dependent clocks */
+ memcpy(dsi->clk_config, dsi->pdata->clk_config,
+ sizeof(dsi->pdata->clk_config));
+
+ for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
+ if (!dsi->clk_config[i].present)
+ continue;
+
+ clk_id = dsi->clk_config[i].id;
+ clk = devm_clk_get(dsi->dev, clk_id);
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ DRM_DEV_ERROR(dsi->dev, "Failed to get %s clock: %d\n",
+ clk_id, ret);
+ return ret;
+ }
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "Setup clk %s (rate: %lu)\n",
+ clk_id, clk_get_rate(clk));
+ dsi->clk_config[i].clk = clk;
+ }
+
+ /* DSI clocks */
+ clk = devm_clk_get(dsi->dev, "phy_ref");
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ DRM_DEV_ERROR(dsi->dev, "Failed to get phy_ref clock: %d\n",
+ ret);
+ return ret;
+ }
+ dsi->phy_ref_clk = clk;
+
+ clk = devm_clk_get(dsi->dev, "rx_esc");
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ DRM_DEV_ERROR(dsi->dev, "Failed to get rx_esc clock: %d\n",
+ ret);
+ return ret;
+ }
+ dsi->rx_esc_clk = clk;
+
+ clk = devm_clk_get(dsi->dev, "tx_esc");
+ if (IS_ERR(clk)) {
+ ret = PTR_ERR(clk);
+ DRM_DEV_ERROR(dsi->dev, "Failed to get tx_esc clock: %d\n",
+ ret);
+ return ret;
+ }
+ dsi->tx_esc_clk = clk;
+
+ dsi->mux_sel = syscon_regmap_lookup_by_phandle(np, "mux-sel");
+ if (IS_ERR(dsi->mux_sel) &&
+ (dsi->pdata->ext_regs & NWL_DSI_IMX_REG_GPR)) {
+ ret = PTR_ERR(dsi->mux_sel);
+ DRM_DEV_ERROR(dsi->dev, "Failed to get GPR regmap: %d\n", ret);
+ return ret;
+ }
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ dsi->regmap =
+ devm_regmap_init_mmio(dsi->dev, base, &nwl_dsi_regmap_config);
+ if (IS_ERR(dsi->regmap)) {
+ ret = PTR_ERR(dsi->regmap);
+ DRM_DEV_ERROR(dsi->dev, "Failed to create NWL DSI regmap: %d\n",
+ ret);
+ return ret;
+ }
+
+ dsi->irq = platform_get_irq(pdev, 0);
+ if (dsi->irq < 0) {
+ DRM_DEV_ERROR(dsi->dev, "Failed to get device IRQ: %d\n",
+ dsi->irq);
+ return dsi->irq;
+ }
+
+ dsi->rstc = devm_reset_control_array_get(dsi->dev, false, true);
+ if (IS_ERR(dsi->rstc)) {
+ DRM_DEV_ERROR(dsi->dev, "Failed to get resets: %ld\n",
+ PTR_ERR(dsi->rstc));
+ return PTR_ERR(dsi->rstc);
+ }
+
+ return 0;
+}
+
+static void imx8mq_dsi_select_input(struct nwl_dsi *dsi)
+{
+ struct device_node *remote;
+ u32 mux_val = IMX8MQ_GPR13_MIPI_MUX_SEL;
+
+ remote = of_graph_get_remote_node(dsi->dev->of_node, 0, 0);
+ if (strcmp(remote->name, "lcdif") == 0)
+ mux_val = 0;
+
+ DRM_DEV_INFO(dsi->dev, "Using %s as input source\n",
+ (mux_val) ? "DCSS" : "LCDIF");
+ regmap_update_bits(dsi->mux_sel, IOMUXC_GPR13,
+ IMX8MQ_GPR13_MIPI_MUX_SEL, mux_val);
+ of_node_put(remote);
+}
+
+static int imx8mq_dsi_poweron(struct nwl_dsi *dsi)
+{
+ int ret = 0;
+
+ /* otherwise the display stays blank */
+ usleep_range(200, 300);
+
+ if (dsi->rstc)
+ ret = reset_control_deassert(dsi->rstc);
+
+ return ret;
+}
+
+static int imx8mq_dsi_poweroff(struct nwl_dsi *dsi)
+{
+ int ret = 0;
+
+ if (dsi->quirks & SRC_RESET_QUIRK)
+ return 0;
+
+ if (dsi->rstc)
+ ret = reset_control_assert(dsi->rstc);
+ return ret;
+}
+
+static const struct drm_bridge_timings nwl_dsi_timings = {
+ .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
+};
+
+static const struct nwl_dsi_platform_data imx8mq_dev = {
+ .poweron = &imx8mq_dsi_poweron,
+ .poweroff = &imx8mq_dsi_poweroff,
+ .select_input = &imx8mq_dsi_select_input,
+ .clk_config = {
+ { .id = NWL_DSI_CLK_CORE, .present = true },
+ },
+ .ext_regs = NWL_DSI_IMX_REG_GPR,
+};
+
+static const struct of_device_id nwl_dsi_dt_ids[] = {
+ { .compatible = "fsl,imx8mq-nwl-dsi", .data = &imx8mq_dev, },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, nwl_dsi_dt_ids);
+
+static const struct soc_device_attribute nwl_dsi_quirks_match[] = {
+ { .soc_id = "i.MX8MQ", .revision = "2.0",
+ .data = (void *)(E11418_HS_MODE_QUIRK | SRC_RESET_QUIRK) },
+ { /* sentinel. */ },
+};
+
+static int nwl_dsi_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct of_device_id *of_id = of_match_device(nwl_dsi_dt_ids, dev);
+ const struct nwl_dsi_platform_data *pdata = of_id->data;
+ const struct soc_device_attribute *attr;
+ struct nwl_dsi *dsi;
+ int ret;
+
+ dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
+ if (!dsi)
+ return -ENOMEM;
+
+ dsi->dev = dev;
+ dsi->pdata = pdata;
+
+ ret = nwl_dsi_parse_dt(dsi);
+ if (ret)
+ return ret;
+
+ ret = devm_request_irq(dev, dsi->irq, nwl_dsi_irq_handler, 0,
+ dev_name(dev), dsi);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to request IRQ %d: %d\n", dsi->irq,
+ ret);
+ return ret;
+ }
+
+ dsi->dsi_host.ops = &nwl_dsi_host_ops;
+ dsi->dsi_host.dev = dev;
+ ret = mipi_dsi_host_register(&dsi->dsi_host);
+ if (ret) {
+ DRM_DEV_ERROR(dev, "Failed to register MIPI host: %d\n", ret);
+ return ret;
+ }
+
+ attr = soc_device_match(nwl_dsi_quirks_match);
+ if (attr)
+ dsi->quirks = (uintptr_t)attr->data;
+
+ dsi->bridge.driver_private = dsi;
+ dsi->bridge.funcs = &nwl_dsi_bridge_funcs;
+ dsi->bridge.of_node = dev->of_node;
+ dsi->bridge.timings = &nwl_dsi_timings;
+
+ drm_bridge_add(&dsi->bridge);
+
+ dev_set_drvdata(dev, dsi);
+ pm_runtime_enable(dev);
+ return 0;
+}
+
+static int nwl_dsi_remove(struct platform_device *pdev)
+{
+ struct nwl_dsi *dsi = platform_get_drvdata(pdev);
+
+ mipi_dsi_host_unregister(&dsi->dsi_host);
+ pm_runtime_disable(&pdev->dev);
+ return 0;
+}
+
+static struct platform_driver nwl_dsi_driver = {
+ .probe = nwl_dsi_probe,
+ .remove = nwl_dsi_remove,
+ .driver = {
+ .of_match_table = nwl_dsi_dt_ids,
+ .name = DRV_NAME,
+ },
+};
+
+module_platform_driver(nwl_dsi_driver);
+
+MODULE_AUTHOR("NXP Semiconductor");
+MODULE_AUTHOR("Purism SPC");
+MODULE_DESCRIPTION("Northwest Logic MIPI-DSI driver");
+MODULE_LICENSE("GPL"); /* GPLv2 or later */
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
new file mode 100644
index 000000000000..65fed25fc9b9
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-drv.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * NWL MIPI DSI host driver
+ *
+ * Copyright (C) 2017 NXP
+ * Copyright (C) 2019 Purism SPC
+ */
+
+#ifndef __NWL_DRV_H__
+#define __NWL_DRV_H__
+
+#include <linux/phy/phy.h>
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_mipi_dsi.h>
+
+struct nwl_dsi_platform_data;
+
+/* i.MX8 NWL quirks */
+/* i.MX8MQ errata E11418 */
+#define E11418_HS_MODE_QUIRK BIT(0)
+/* Skip DSI bits in SRC on disable to avoid blank display on enable */
+#define SRC_RESET_QUIRK BIT(1)
+
+#define NWL_DSI_MAX_PLATFORM_CLOCKS 1
+struct nwl_dsi_plat_clk_config {
+ const char *id;
+ struct clk *clk;
+ bool present;
+};
+
+struct nwl_dsi {
+ struct drm_bridge bridge;
+ struct mipi_dsi_host dsi_host;
+ struct drm_bridge *panel_bridge;
+ struct device *dev;
+ struct phy *phy;
+ union phy_configure_opts phy_cfg;
+ unsigned int quirks;
+
+ struct regmap *regmap;
+ int irq;
+ struct reset_control *rstc;
+
+ /* External registers */
+ struct regmap *mux_sel;
+
+ /* DSI clocks */
+ struct clk *phy_ref_clk;
+ struct clk *rx_esc_clk;
+ struct clk *tx_esc_clk;
+ /* Platform dependent clocks */
+ struct nwl_dsi_plat_clk_config clk_config[NWL_DSI_MAX_PLATFORM_CLOCKS];
+
+ /* dsi lanes */
+ u32 lanes;
+ enum mipi_dsi_pixel_format format;
+ struct drm_display_mode mode;
+ unsigned long dsi_mode_flags;
+
+ struct nwl_dsi_transfer *xfer;
+
+ const struct nwl_dsi_platform_data *pdata;
+};
+
+#endif /* __NWL_DRV_H__ */
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
new file mode 100644
index 000000000000..fd030af55bb4
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.c
@@ -0,0 +1,700 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NWL MIPI DSI host driver
+ *
+ * Copyright (C) 2017 NXP
+ * Copyright (C) 2019 Purism SPC
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/irq.h>
+#include <linux/regmap.h>
+#include <linux/time64.h>
+
+#include <video/mipi_display.h>
+#include <video/videomode.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_of.h>
+#include <drm/drm_panel.h>
+#include <drm/drm_print.h>
+
+#include "nwl-drv.h"
+#include "nwl-dsi.h"
+
+#define NWL_DSI_MIPI_FIFO_TIMEOUT msecs_to_jiffies(500)
+
+/*
+ * PKT_CONTROL format:
+ * [15: 0] - word count
+ * [17:16] - virtual channel
+ * [23:18] - data type
+ * [24] - LP or HS select (0 - LP, 1 - HS)
+ * [25] - perform BTA after packet is sent
+ * [26] - perform BTA only, no packet tx
+ */
+#define NWL_DSI_WC(x) FIELD_PREP(GENMASK(15, 0), (x))
+#define NWL_DSI_TX_VC(x) FIELD_PREP(GENMASK(17, 16), (x))
+#define NWL_DSI_TX_DT(x) FIELD_PREP(GENMASK(23, 18), (x))
+#define NWL_DSI_HS_SEL(x) FIELD_PREP(GENMASK(24, 24), (x))
+#define NWL_DSI_BTA_TX(x) FIELD_PREP(GENMASK(25, 25), (x))
+#define NWL_DSI_BTA_NO_TX(x) FIELD_PREP(GENMASK(26, 26), (x))
+
+/*
+ * RX_PKT_HEADER format:
+ * [15: 0] - word count
+ * [21:16] - data type
+ * [23:22] - virtual channel
+ */
+#define NWL_DSI_RX_DT(x) FIELD_GET(GENMASK(21, 16), (x))
+#define NWL_DSI_RX_VC(x) FIELD_GET(GENMASK(23, 22), (x))
+
+/* DSI Video mode */
+#define NWL_DSI_VM_BURST_MODE_WITH_SYNC_PULSES 0
+#define NWL_DSI_VM_NON_BURST_MODE_WITH_SYNC_EVENTS BIT(0)
+#define NWL_DSI_VM_BURST_MODE BIT(1)
+
+/* * DPI color coding */
+#define NWL_DSI_DPI_16_BIT_565_PACKED 0
+#define NWL_DSI_DPI_16_BIT_565_ALIGNED 1
+#define NWL_DSI_DPI_16_BIT_565_SHIFTED 2
+#define NWL_DSI_DPI_18_BIT_PACKED 3
+#define NWL_DSI_DPI_18_BIT_ALIGNED 4
+#define NWL_DSI_DPI_24_BIT 5
+
+/* * DPI Pixel format */
+#define NWL_DSI_PIXEL_FORMAT_16 0
+#define NWL_DSI_PIXEL_FORMAT_18 BIT(0)
+#define NWL_DSI_PIXEL_FORMAT_18L BIT(1)
+#define NWL_DSI_PIXEL_FORMAT_24 (BIT(0) | BIT(1))
+
+enum transfer_direction {
+ DSI_PACKET_SEND,
+ DSI_PACKET_RECEIVE,
+};
+
+struct nwl_dsi_transfer {
+ const struct mipi_dsi_msg *msg;
+ struct mipi_dsi_packet packet;
+ struct completion completed;
+
+ int status; /* status of transmission */
+ enum transfer_direction direction;
+ bool need_bta;
+ u8 cmd;
+ u16 rx_word_count;
+ size_t tx_len; /* in bytes */
+ size_t rx_len; /* in bytes */
+};
+
+static int nwl_dsi_write(struct nwl_dsi *dsi, unsigned int reg, u32 val)
+{
+ int ret;
+
+ ret = regmap_write(dsi->regmap, reg, val);
+ if (ret < 0)
+ DRM_DEV_ERROR(dsi->dev,
+ "Failed to write NWL DSI reg 0x%x: %d\n", reg,
+ ret);
+ return ret;
+}
+
+static u32 nwl_dsi_read(struct nwl_dsi *dsi, u32 reg)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(dsi->regmap, reg, &val);
+ if (ret < 0)
+ DRM_DEV_ERROR(dsi->dev, "Failed to read NWL DSI reg 0x%x: %d\n",
+ reg, ret);
+
+ return val;
+}
+
+static u32 nwl_dsi_get_dpi_pixel_format(enum mipi_dsi_pixel_format format)
+{
+ switch (format) {
+ case MIPI_DSI_FMT_RGB565:
+ return NWL_DSI_PIXEL_FORMAT_16;
+ case MIPI_DSI_FMT_RGB666:
+ return NWL_DSI_PIXEL_FORMAT_18L;
+ case MIPI_DSI_FMT_RGB666_PACKED:
+ return NWL_DSI_PIXEL_FORMAT_18;
+ case MIPI_DSI_FMT_RGB888:
+ return NWL_DSI_PIXEL_FORMAT_24;
+ default:
+ return -EINVAL;
+ }
+}
+
+#define PSEC_PER_SEC 1000000000000LL
+/*
+ * ps2bc - Picoseconds to byte clock cycles
+ */
+static u32 ps2bc(struct nwl_dsi *dsi, unsigned long long ps)
+{
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ return DIV_ROUND_UP(ps * dsi->mode.clock * 1000 * bpp,
+ dsi->lanes * 8 * PSEC_PER_SEC);
+}
+
+/*
+ * ui2bc - UI time periods to byte clock cycles
+ */
+static u32 ui2bc(struct nwl_dsi *dsi, unsigned long long ui)
+{
+ int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
+
+ return DIV_ROUND_UP(ui * dsi->lanes, dsi->mode.clock * 1000 * bpp);
+}
+
+/*
+ * us2bc - micro seconds to lp clock cycles
+ */
+static u32 us2lp(u32 lp_clk_rate, unsigned long us)
+{
+ return DIV_ROUND_UP(us * lp_clk_rate, USEC_PER_SEC);
+}
+
+static int nwl_dsi_config_host(struct nwl_dsi *dsi)
+{
+ u32 cycles;
+ struct phy_configure_opts_mipi_dphy *cfg = &dsi->phy_cfg.mipi_dphy;
+
+ if (dsi->lanes < 1 || dsi->lanes > 4)
+ return -EINVAL;
+
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "DSI Lanes %d\n", dsi->lanes);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_NUM_LANES, dsi->lanes - 1);
+
+ if (dsi->dsi_mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
+ nwl_dsi_write(dsi, NWL_DSI_CFG_NONCONTINUOUS_CLK, 0x01);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_AUTOINSERT_EOTP, 0x01);
+ } else {
+ nwl_dsi_write(dsi, NWL_DSI_CFG_NONCONTINUOUS_CLK, 0x00);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_AUTOINSERT_EOTP, 0x00);
+ }
+
+ /* values in byte clock cycles */
+ cycles = ui2bc(dsi, cfg->clk_pre);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_T_PRE, cycles);
+ cycles = ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg->clk_zero);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n", cycles);
+ cycles += ui2bc(dsi, cfg->clk_pre);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_T_POST, cycles);
+ cycles = ps2bc(dsi, cfg->hs_exit);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_TX_GAP, cycles);
+
+ nwl_dsi_write(dsi, NWL_DSI_CFG_EXTRA_CMDS_AFTER_EOTP, 0x01);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_HTX_TO_COUNT, 0x00);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_LRX_H_TO_COUNT, 0x00);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_BTA_H_TO_COUNT, 0x00);
+ /* In LP clock cycles */
+ cycles = us2lp(cfg->lp_clk_rate, cfg->wakeup);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_twakeup: 0x%x\n", cycles);
+ nwl_dsi_write(dsi, NWL_DSI_CFG_TWAKEUP, cycles);
+
+ return 0;
+}
+
+static int nwl_dsi_config_dpi(struct nwl_dsi *dsi)
+{
+ u32 color_format, mode;
+ bool burst_mode;
+ int hfront_porch, hback_porch, vfront_porch, vback_porch;
+ int hsync_len, vsync_len;
+
+ hfront_porch = dsi->mode.hsync_start - dsi->mode.hdisplay;
+ hsync_len = dsi->mode.hsync_end - dsi->mode.hsync_start;
+ hback_porch = dsi->mode.htotal - dsi->mode.hsync_end;
+
+ vfront_porch = dsi->mode.vsync_start - dsi->mode.vdisplay;
+ vsync_len = dsi->mode.vsync_end - dsi->mode.vsync_start;
+ vback_porch = dsi->mode.vtotal - dsi->mode.vsync_end;
+
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "hfront_porch = %d\n", hfront_porch);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "hback_porch = %d\n", hback_porch);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "hsync_len = %d\n", hsync_len);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "hdisplay = %d\n", dsi->mode.hdisplay);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "vfront_porch = %d\n", vfront_porch);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "vback_porch = %d\n", vback_porch);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "vsync_len = %d\n", vsync_len);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "vactive = %d\n", dsi->mode.vdisplay);
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "clock = %d kHz\n", dsi->mode.clock);
+
+ color_format = nwl_dsi_get_dpi_pixel_format(dsi->format);
+ if (color_format < 0) {
+ DRM_DEV_ERROR(dsi->dev, "Invalid color format 0x%x\n",
+ dsi->format);
+ return color_format;
+ }
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "pixel fmt = %d\n", dsi->format);
+
+ nwl_dsi_write(dsi, NWL_DSI_INTERFACE_COLOR_CODING, NWL_DSI_DPI_24_BIT);
+ nwl_dsi_write(dsi, NWL_DSI_PIXEL_FORMAT, color_format);
+ /*
+ * Adjusting input polarity based on the video mode results in
+ * a black screen so always pick active low:
+ */
+ nwl_dsi_write(dsi, NWL_DSI_VSYNC_POLARITY,
+ NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW);
+ nwl_dsi_write(dsi, NWL_DSI_HSYNC_POLARITY,
+ NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW);
+
+ burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
+ !(dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE);
+
+ if (burst_mode) {
+ nwl_dsi_write(dsi, NWL_DSI_VIDEO_MODE, NWL_DSI_VM_BURST_MODE);
+ nwl_dsi_write(dsi, NWL_DSI_PIXEL_FIFO_SEND_LEVEL, 256);
+ } else {
+ mode = ((dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) ?
+ NWL_DSI_VM_BURST_MODE_WITH_SYNC_PULSES :
+ NWL_DSI_VM_NON_BURST_MODE_WITH_SYNC_EVENTS);
+ nwl_dsi_write(dsi, NWL_DSI_VIDEO_MODE, mode);
+ nwl_dsi_write(dsi, NWL_DSI_PIXEL_FIFO_SEND_LEVEL,
+ dsi->mode.hdisplay);
+ }
+
+ nwl_dsi_write(dsi, NWL_DSI_HFP, hfront_porch);
+ nwl_dsi_write(dsi, NWL_DSI_HBP, hback_porch);
+ nwl_dsi_write(dsi, NWL_DSI_HSA, hsync_len);
+
+ nwl_dsi_write(dsi, NWL_DSI_ENABLE_MULT_PKTS, 0x0);
+ nwl_dsi_write(dsi, NWL_DSI_BLLP_MODE, 0x1);
+ nwl_dsi_write(dsi, NWL_DSI_ENABLE_MULT_PKTS, 0x0);
+ nwl_dsi_write(dsi, NWL_DSI_USE_NULL_PKT_BLLP, 0x0);
+ nwl_dsi_write(dsi, NWL_DSI_VC, 0x0);
+
+ nwl_dsi_write(dsi, NWL_DSI_PIXEL_PAYLOAD_SIZE, dsi->mode.hdisplay);
+ nwl_dsi_write(dsi, NWL_DSI_VACTIVE, dsi->mode.vdisplay - 1);
+ nwl_dsi_write(dsi, NWL_DSI_VBP, vback_porch);
+ nwl_dsi_write(dsi, NWL_DSI_VFP, vfront_porch);
+
+ return 0;
+}
+
+static void nwl_dsi_init_interrupts(struct nwl_dsi *dsi)
+{
+ u32 irq_enable;
+
+ nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK, 0xffffffff);
+ nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK2, 0x7);
+
+ irq_enable = ~(u32)(NWL_DSI_TX_PKT_DONE_MASK |
+ NWL_DSI_RX_PKT_HDR_RCVD_MASK |
+ NWL_DSI_TX_FIFO_OVFLW_MASK |
+ NWL_DSI_HS_TX_TIMEOUT_MASK);
+
+ nwl_dsi_write(dsi, NWL_DSI_IRQ_MASK, irq_enable);
+}
+
+static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
+ struct mipi_dsi_device *device)
+{
+ struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
+ struct device *dev = dsi->dev;
+ struct drm_bridge *bridge;
+ struct drm_panel *panel;
+ int ret;
+
+ DRM_DEV_INFO(dev, "lanes=%u, format=0x%x flags=0x%lx\n", device->lanes,
+ device->format, device->mode_flags);
+
+ if (device->lanes < 1 || device->lanes > 4)
+ return -EINVAL;
+
+ dsi->lanes = device->lanes;
+ dsi->format = device->format;
+ dsi->dsi_mode_flags = device->mode_flags;
+
+ ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
+ &bridge);
+ if (ret)
+ return ret;
+
+ if (panel) {
+ bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
+ if (IS_ERR(bridge))
+ return PTR_ERR(bridge);
+ }
+
+ dsi->panel_bridge = bridge;
+ drm_bridge_add(&dsi->bridge);
+
+ return 0;
+}
+
+static int nwl_dsi_host_detach(struct mipi_dsi_host *dsi_host,
+ struct mipi_dsi_device *device)
+{
+ struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
+
+ drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
+ drm_bridge_remove(&dsi->bridge);
+
+ return 0;
+}
+
+static bool nwl_dsi_read_packet(struct nwl_dsi *dsi, u32 status)
+{
+ struct device *dev = dsi->dev;
+ struct nwl_dsi_transfer *xfer = dsi->xfer;
+ u8 *payload = xfer->msg->rx_buf;
+ u32 val;
+ u16 word_count;
+ u8 channel;
+ u8 data_type;
+
+ xfer->status = 0;
+
+ if (xfer->rx_word_count == 0) {
+ if (!(status & NWL_DSI_RX_PKT_HDR_RCVD))
+ return false;
+ /* Get the RX header and parse it */
+ val = nwl_dsi_read(dsi, NWL_DSI_RX_PKT_HEADER);
+ word_count = NWL_DSI_WC(val);
+ channel = NWL_DSI_RX_VC(val);
+ data_type = NWL_DSI_RX_DT(val);
+
+ if (channel != xfer->msg->channel) {
+ DRM_DEV_ERROR(dev,
+ "[%02X] Channel mismatch (%u != %u)\n",
+ xfer->cmd, channel, xfer->msg->channel);
+ return true;
+ }
+
+ switch (data_type) {
+ case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
+ /* Fall through */
+ case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
+ if (xfer->msg->rx_len > 1) {
+ /* read second byte */
+ payload[1] = word_count >> 8;
+ ++xfer->rx_len;
+ }
+ /* Fall through */
+ case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
+ /* Fall through */
+ case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
+ if (xfer->msg->rx_len > 0) {
+ /* read first byte */
+ payload[0] = word_count & 0xff;
+ ++xfer->rx_len;
+ }
+ xfer->status = xfer->rx_len;
+ return true;
+ case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
+ word_count &= 0xff;
+ DRM_DEV_ERROR(dev, "[%02X] DSI error report: 0x%02x\n",
+ xfer->cmd, word_count);
+ xfer->status = -EPROTO;
+ return true;
+ }
+
+ if (word_count > xfer->msg->rx_len) {
+ DRM_DEV_ERROR(
+ dev,
+ "[%02X] Receive buffer too small: %lu (< %u)\n",
+ xfer->cmd, xfer->msg->rx_len, word_count);
+ return true;
+ }
+
+ xfer->rx_word_count = word_count;
+ } else {
+ /* Set word_count from previous header read */
+ word_count = xfer->rx_word_count;
+ }
+
+ /* If RX payload is not yet received, wait for it */
+ if (!(status & NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD))
+ return false;
+
+ /* Read the RX payload */
+ while (word_count >= 4) {
+ val = nwl_dsi_read(dsi, NWL_DSI_RX_PAYLOAD);
+ payload[0] = (val >> 0) & 0xff;
+ payload[1] = (val >> 8) & 0xff;
+ payload[2] = (val >> 16) & 0xff;
+ payload[3] = (val >> 24) & 0xff;
+ payload += 4;
+ xfer->rx_len += 4;
+ word_count -= 4;
+ }
+
+ if (word_count > 0) {
+ val = nwl_dsi_read(dsi, NWL_DSI_RX_PAYLOAD);
+ switch (word_count) {
+ case 3:
+ payload[2] = (val >> 16) & 0xff;
+ ++xfer->rx_len;
+ /* Fall through */
+ case 2:
+ payload[1] = (val >> 8) & 0xff;
+ ++xfer->rx_len;
+ /* Fall through */
+ case 1:
+ payload[0] = (val >> 0) & 0xff;
+ ++xfer->rx_len;
+ break;
+ }
+ }
+
+ xfer->status = xfer->rx_len;
+
+ return true;
+}
+
+static void nwl_dsi_finish_transmission(struct nwl_dsi *dsi, u32 status)
+{
+ struct nwl_dsi_transfer *xfer = dsi->xfer;
+ bool end_packet = false;
+
+ if (!xfer)
+ return;
+
+ if (status & NWL_DSI_TX_FIFO_OVFLW) {
+ DRM_DEV_ERROR_RATELIMITED(dsi->dev, "tx fifo overflow\n");
+ return;
+ }
+
+ if (status & NWL_DSI_HS_TX_TIMEOUT) {
+ DRM_DEV_ERROR_RATELIMITED(dsi->dev, "HS tx timeout\n");
+ return;
+ }
+
+ if (xfer->direction == DSI_PACKET_SEND &&
+ status & NWL_DSI_TX_PKT_DONE) {
+ xfer->status = xfer->tx_len;
+ end_packet = true;
+ } else if (status & NWL_DSI_DPHY_DIRECTION &&
+ ((status & (NWL_DSI_RX_PKT_HDR_RCVD |
+ NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD)))) {
+ end_packet = nwl_dsi_read_packet(dsi, status);
+ }
+
+ if (end_packet)
+ complete(&xfer->completed);
+}
+
+static void nwl_dsi_begin_transmission(struct nwl_dsi *dsi)
+{
+ struct nwl_dsi_transfer *xfer = dsi->xfer;
+ struct mipi_dsi_packet *pkt = &xfer->packet;
+ const u8 *payload;
+ size_t length;
+ u16 word_count;
+ u8 hs_mode;
+ u32 val;
+ u32 hs_workaround = 0;
+
+ /* Send the payload, if any */
+ length = pkt->payload_length;
+ payload = pkt->payload;
+
+ while (length >= 4) {
+ val = *(u32 *)payload;
+ hs_workaround |= !(val & 0xFFFF00);
+ nwl_dsi_write(dsi, NWL_DSI_TX_PAYLOAD, val);
+ payload += 4;
+ length -= 4;
+ }
+ /* Send the rest of the payload */
+ val = 0;
+ switch (length) {
+ case 3:
+ val |= payload[2] << 16;
+ /* Fall through */
+ case 2:
+ val |= payload[1] << 8;
+ hs_workaround |= !(val & 0xFFFF00);
+ /* Fall through */
+ case 1:
+ val |= payload[0];
+ nwl_dsi_write(dsi, NWL_DSI_TX_PAYLOAD, val);
+ break;
+ }
+ xfer->tx_len = pkt->payload_length;
+
+ /*
+ * Send the header
+ * header[0] = Virtual Channel + Data Type
+ * header[1] = Word Count LSB (LP) or first param (SP)
+ * header[2] = Word Count MSB (LP) or second param (SP)
+ */
+ word_count = pkt->header[1] | (pkt->header[2] << 8);
+ if (hs_workaround && (dsi->quirks & E11418_HS_MODE_QUIRK)) {
+ DRM_DEV_DEBUG_DRIVER(dsi->dev,
+ "Using hs mode workaround for cmd 0x%x\n",
+ xfer->cmd);
+ hs_mode = 1;
+ } else {
+ hs_mode = (xfer->msg->flags & MIPI_DSI_MSG_USE_LPM) ? 0 : 1;
+ }
+ val = NWL_DSI_WC(word_count) | NWL_DSI_TX_VC(xfer->msg->channel) |
+ NWL_DSI_TX_DT(xfer->msg->type) | NWL_DSI_HS_SEL(hs_mode) |
+ NWL_DSI_BTA_TX(xfer->need_bta);
+ nwl_dsi_write(dsi, NWL_DSI_PKT_CONTROL, val);
+
+ /* Send packet command */
+ nwl_dsi_write(dsi, NWL_DSI_SEND_PACKET, 0x1);
+}
+
+static ssize_t nwl_dsi_host_transfer(struct mipi_dsi_host *dsi_host,
+ const struct mipi_dsi_msg *msg)
+{
+ struct nwl_dsi *dsi = container_of(dsi_host, struct nwl_dsi, dsi_host);
+ struct nwl_dsi_transfer xfer;
+ ssize_t ret = 0;
+
+ /* Create packet to be sent */
+ dsi->xfer = &xfer;
+ ret = mipi_dsi_create_packet(&xfer.packet, msg);
+ if (ret < 0) {
+ dsi->xfer = NULL;
+ return ret;
+ }
+
+ if ((msg->type & MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM ||
+ msg->type & MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM ||
+ msg->type & MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM ||
+ msg->type & MIPI_DSI_DCS_READ) &&
+ msg->rx_len > 0 && msg->rx_buf != NULL)
+ xfer.direction = DSI_PACKET_RECEIVE;
+ else
+ xfer.direction = DSI_PACKET_SEND;
+
+ xfer.need_bta = (xfer.direction == DSI_PACKET_RECEIVE);
+ xfer.need_bta |= (msg->flags & MIPI_DSI_MSG_REQ_ACK) ? 1 : 0;
+ xfer.msg = msg;
+ xfer.status = -ETIMEDOUT;
+ xfer.rx_word_count = 0;
+ xfer.rx_len = 0;
+ xfer.cmd = 0x00;
+ if (msg->tx_len > 0)
+ xfer.cmd = ((u8 *)(msg->tx_buf))[0];
+ init_completion(&xfer.completed);
+
+ ret = clk_prepare_enable(dsi->rx_esc_clk);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dsi->dev, "Failed to enable rx_esc clk: %zd\n",
+ ret);
+ return ret;
+ }
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "Enabled rx_esc clk @%lu Hz\n",
+ clk_get_rate(dsi->rx_esc_clk));
+
+ /* Initiate the DSI packet transmision */
+ nwl_dsi_begin_transmission(dsi);
+
+ if (!wait_for_completion_timeout(&xfer.completed,
+ NWL_DSI_MIPI_FIFO_TIMEOUT)) {
+ DRM_DEV_ERROR(dsi_host->dev, "[%02X] DSI transfer timed out\n",
+ xfer.cmd);
+ ret = -ETIMEDOUT;
+ } else {
+ ret = xfer.status;
+ }
+
+ clk_disable_unprepare(dsi->rx_esc_clk);
+
+ return ret;
+}
+
+const struct mipi_dsi_host_ops nwl_dsi_host_ops = {
+ .attach = nwl_dsi_host_attach,
+ .detach = nwl_dsi_host_detach,
+ .transfer = nwl_dsi_host_transfer,
+};
+
+irqreturn_t nwl_dsi_irq_handler(int irq, void *data)
+{
+ u32 irq_status;
+ struct nwl_dsi *dsi = data;
+
+ irq_status = nwl_dsi_read(dsi, NWL_DSI_IRQ_STATUS);
+
+ if (irq_status & NWL_DSI_TX_PKT_DONE ||
+ irq_status & NWL_DSI_RX_PKT_HDR_RCVD ||
+ irq_status & NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD)
+ nwl_dsi_finish_transmission(dsi, irq_status);
+
+ return IRQ_HANDLED;
+}
+
+int nwl_dsi_enable(struct nwl_dsi *dsi)
+{
+ struct device *dev = dsi->dev;
+ union phy_configure_opts *phy_cfg = &dsi->phy_cfg;
+ int ret;
+
+ if (!dsi->lanes) {
+ DRM_DEV_ERROR(dev, "Need DSI lanes: %d\n", dsi->lanes);
+ return -EINVAL;
+ }
+
+ ret = phy_init(dsi->phy);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to init DSI phy: %d\n", ret);
+ return ret;
+ }
+
+ ret = phy_configure(dsi->phy, phy_cfg);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to configure DSI phy: %d\n", ret);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(dsi->tx_esc_clk);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dsi->dev, "Failed to enable tx_esc clk: %d\n",
+ ret);
+ return ret;
+ }
+ DRM_DEV_DEBUG_DRIVER(dsi->dev, "Enabled tx_esc clk @%lu Hz\n",
+ clk_get_rate(dsi->tx_esc_clk));
+
+ ret = nwl_dsi_config_host(dsi);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to set up DSI: %d", ret);
+ return ret;
+ }
+
+ ret = nwl_dsi_config_dpi(dsi);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to set up DPI: %d", ret);
+ return ret;
+ }
+
+ ret = phy_power_on(dsi->phy);
+ if (ret < 0) {
+ DRM_DEV_ERROR(dev, "Failed to power on DPHY (%d)\n", ret);
+ return ret;
+ }
+
+ nwl_dsi_init_interrupts(dsi);
+
+ return 0;
+}
+
+int nwl_dsi_disable(struct nwl_dsi *dsi)
+{
+ struct device *dev = dsi->dev;
+
+ DRM_DEV_DEBUG_DRIVER(dev, "Disabling clocks and phy\n");
+
+ phy_power_off(dsi->phy);
+ phy_exit(dsi->phy);
+
+ /* Disabling the clock before the phy breaks enabling dsi again */
+ clk_disable_unprepare(dsi->tx_esc_clk);
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
new file mode 100644
index 000000000000..579b366de652
--- /dev/null
+++ b/drivers/gpu/drm/bridge/nwl-dsi/nwl-dsi.h
@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * NWL MIPI DSI host driver
+ *
+ * Copyright (C) 2017 NXP
+ * Copyright (C) 2019 Purism SPC
+ */
+#ifndef __NWL_DSI_H__
+#define __NWL_DSI_H__
+
+#include <linux/irqreturn.h>
+
+#include <drm/drm_mipi_dsi.h>
+
+#include "nwl-drv.h"
+
+/* DSI HOST registers */
+#define NWL_DSI_CFG_NUM_LANES 0x0
+#define NWL_DSI_CFG_NONCONTINUOUS_CLK 0x4
+#define NWL_DSI_CFG_T_PRE 0x8
+#define NWL_DSI_CFG_T_POST 0xc
+#define NWL_DSI_CFG_TX_GAP 0x10
+#define NWL_DSI_CFG_AUTOINSERT_EOTP 0x14
+#define NWL_DSI_CFG_EXTRA_CMDS_AFTER_EOTP 0x18
+#define NWL_DSI_CFG_HTX_TO_COUNT 0x1c
+#define NWL_DSI_CFG_LRX_H_TO_COUNT 0x20
+#define NWL_DSI_CFG_BTA_H_TO_COUNT 0x24
+#define NWL_DSI_CFG_TWAKEUP 0x28
+#define NWL_DSI_CFG_STATUS_OUT 0x2c
+#define NWL_DSI_RX_ERROR_STATUS 0x30
+
+/* DSI DPI registers */
+#define NWL_DSI_PIXEL_PAYLOAD_SIZE 0x200
+#define NWL_DSI_PIXEL_FIFO_SEND_LEVEL 0x204
+#define NWL_DSI_INTERFACE_COLOR_CODING 0x208
+#define NWL_DSI_PIXEL_FORMAT 0x20c
+#define NWL_DSI_VSYNC_POLARITY 0x210
+#define NWL_DSI_VSYNC_POLARITY_ACTIVE_LOW 0
+#define NWL_DSI_VSYNC_POLARITY_ACTIVE_HIGH BIT(1)
+
+#define NWL_DSI_HSYNC_POLARITY 0x214
+#define NWL_DSI_HSYNC_POLARITY_ACTIVE_LOW 0
+#define NWL_DSI_HSYNC_POLARITY_ACTIVE_HIGH BIT(1)
+
+#define NWL_DSI_VIDEO_MODE 0x218
+#define NWL_DSI_HFP 0x21c
+#define NWL_DSI_HBP 0x220
+#define NWL_DSI_HSA 0x224
+#define NWL_DSI_ENABLE_MULT_PKTS 0x228
+#define NWL_DSI_VBP 0x22c
+#define NWL_DSI_VFP 0x230
+#define NWL_DSI_BLLP_MODE 0x234
+#define NWL_DSI_USE_NULL_PKT_BLLP 0x238
+#define NWL_DSI_VACTIVE 0x23c
+#define NWL_DSI_VC 0x240
+
+/* DSI APB PKT control */
+#define NWL_DSI_TX_PAYLOAD 0x280
+#define NWL_DSI_PKT_CONTROL 0x284
+#define NWL_DSI_SEND_PACKET 0x288
+#define NWL_DSI_PKT_STATUS 0x28c
+#define NWL_DSI_PKT_FIFO_WR_LEVEL 0x290
+#define NWL_DSI_PKT_FIFO_RD_LEVEL 0x294
+#define NWL_DSI_RX_PAYLOAD 0x298
+#define NWL_DSI_RX_PKT_HEADER 0x29c
+
+/* DSI IRQ handling */
+#define NWL_DSI_IRQ_STATUS 0x2a0
+#define NWL_DSI_SM_NOT_IDLE BIT(0)
+#define NWL_DSI_TX_PKT_DONE BIT(1)
+#define NWL_DSI_DPHY_DIRECTION BIT(2)
+#define NWL_DSI_TX_FIFO_OVFLW BIT(3)
+#define NWL_DSI_TX_FIFO_UDFLW BIT(4)
+#define NWL_DSI_RX_FIFO_OVFLW BIT(5)
+#define NWL_DSI_RX_FIFO_UDFLW BIT(6)
+#define NWL_DSI_RX_PKT_HDR_RCVD BIT(7)
+#define NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD BIT(8)
+#define NWL_DSI_BTA_TIMEOUT BIT(29)
+#define NWL_DSI_LP_RX_TIMEOUT BIT(30)
+#define NWL_DSI_HS_TX_TIMEOUT BIT(31)
+
+#define NWL_DSI_IRQ_STATUS2 0x2a4
+#define NWL_DSI_SINGLE_BIT_ECC_ERR BIT(0)
+#define NWL_DSI_MULTI_BIT_ECC_ERR BIT(1)
+#define NWL_DSI_CRC_ERR BIT(2)
+
+#define NWL_DSI_IRQ_MASK 0x2a8
+#define NWL_DSI_SM_NOT_IDLE_MASK BIT(0)
+#define NWL_DSI_TX_PKT_DONE_MASK BIT(1)
+#define NWL_DSI_DPHY_DIRECTION_MASK BIT(2)
+#define NWL_DSI_TX_FIFO_OVFLW_MASK BIT(3)
+#define NWL_DSI_TX_FIFO_UDFLW_MASK BIT(4)
+#define NWL_DSI_RX_FIFO_OVFLW_MASK BIT(5)
+#define NWL_DSI_RX_FIFO_UDFLW_MASK BIT(6)
+#define NWL_DSI_RX_PKT_HDR_RCVD_MASK BIT(7)
+#define NWL_DSI_RX_PKT_PAYLOAD_DATA_RCVD_MASK BIT(8)
+#define NWL_DSI_BTA_TIMEOUT_MASK BIT(29)
+#define NWL_DSI_LP_RX_TIMEOUT_MASK BIT(30)
+#define NWL_DSI_HS_TX_TIMEOUT_MASK BIT(31)
+
+#define NWL_DSI_IRQ_MASK2 0x2ac
+#define NWL_DSI_SINGLE_BIT_ECC_ERR_MASK BIT(0)
+#define NWL_DSI_MULTI_BIT_ECC_ERR_MASK BIT(1)
+#define NWL_DSI_CRC_ERR_MASK BIT(2)
+
+extern const struct mipi_dsi_host_ops nwl_dsi_host_ops;
+
+irqreturn_t nwl_dsi_irq_handler(int irq, void *data);
+int nwl_dsi_enable(struct nwl_dsi *dsi);
+int nwl_dsi_disable(struct nwl_dsi *dsi);
+
+#endif /* __NWL_DSI_H__ */
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 3/3] drm/bridge: Add NWL MIPI DSI host controller support
From: Guido Günther @ 2019-08-09 16:25 UTC (permalink / raw)
To: Laurent Pinchart
Cc: David Airlie, Daniel Vetter, Rob Herring, Mark Rutland, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Andrzej Hajda, Neil Armstrong, Jonas Karlman,
Jernej Skrabec, Lee Jones, dri-devel, devicetree,
linux-arm-kernel, linux-kernel, Robert Chiras
In-Reply-To: <20190727024700.GD4902@pendragon.ideasonboard.com>
Hi Laurent,
thanks for the review! Most of it seemed clear how to fix for the rest
i've put some questions below:
On Sat, Jul 27, 2019 at 05:47:00AM +0300, Laurent Pinchart wrote:
> Hello Guido,
>
> Thank you for the patch.
>
> On Wed, Jul 24, 2019 at 05:52:26PM +0200, Guido Günther wrote:
> > This adds initial support for the NWL MIPI DSI Host controller found on
> > i.MX8 SoCs.
> >
> > It adds support for the i.MX8MQ but the same IP can be found on
> > e.g. the i.MX8QXP.
> >
> > It has been tested on the Librem 5 devkit using mxsfb.
> >
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > Co-developed-by: Robert Chiras <robert.chiras@nxp.com>
> > ---
> > drivers/gpu/drm/bridge/Kconfig | 2 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/imx-nwl/Kconfig | 15 +
> > drivers/gpu/drm/bridge/imx-nwl/Makefile | 2 +
> > drivers/gpu/drm/bridge/imx-nwl/nwl-drv.c | 529 ++++++++++++++++
> > drivers/gpu/drm/bridge/imx-nwl/nwl-drv.h | 72 +++
> > drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.c | 745 +++++++++++++++++++++++
> > drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.h | 111 ++++
> > 8 files changed, 1477 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/Kconfig
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/Makefile
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/nwl-drv.c
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/nwl-drv.h
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.c
> > create mode 100644 drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.h
> >
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index a6eec908c43e..38c3145a7e57 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -152,6 +152,8 @@ source "drivers/gpu/drm/bridge/analogix/Kconfig"
> >
> > source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> >
> > +source "drivers/gpu/drm/bridge/imx-nwl/Kconfig"
> > +
>
> As this doesn't seem to be an i.MX-specific IP, I wouldn't use the name
> imx in file names or in the code, at least in the parts that are not
> NXP-specific.
O.k. Since i've not seen other SoCs using this ip core I wasn't sure
what would be sharable but we'll figure that out. Renamed to nwl-dsi/
> > source "drivers/gpu/drm/bridge/synopsys/Kconfig"
> >
> > endmenu
> > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> > index 4934fcf5a6f8..904a9eb3a20a 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -16,4 +16,5 @@ obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
> > obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
> > obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> > +obj-y += imx-nwl/
> > obj-y += synopsys/
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/Kconfig b/drivers/gpu/drm/bridge/imx-nwl/Kconfig
> > new file mode 100644
> > index 000000000000..822dba1b380a
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/Kconfig
> > @@ -0,0 +1,15 @@
> > +config DRM_IMX_NWL_DSI
> > + tristate "Support for Northwest Logic MIPI DSI Host controller"
> > + depends on DRM && (ARCH_MXC || ARCH_MULTIPLATFORM || COMPILE_TEST)
> > + depends on COMMON_CLK
> > + depends on OF && HAS_IOMEM
> > + select DRM_KMS_HELPER
> > + select DRM_MIPI_DSI
> > + select DRM_PANEL_BRIDGE
> > + select GENERIC_PHY_MIPI_DPHY
> > + select MFD_SYSCON
> > + select REGMAP_MMIO
> > + help
> > + This enables the Northwest Logic MIPI DSI Host controller as
> > + found on NXP's i.MX8 Processors.
> > +
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/Makefile b/drivers/gpu/drm/bridge/imx-nwl/Makefile
> > new file mode 100644
> > index 000000000000..9fa63483da5b
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/Makefile
> > @@ -0,0 +1,2 @@
> > +imx-nwl-objs := nwl-drv.o nwl-dsi.o
> > +obj-$(CONFIG_DRM_IMX_NWL_DSI) += imx-nwl.o
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.c b/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.c
> > new file mode 100644
> > index 000000000000..451f8f067c6f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.c
> > @@ -0,0 +1,529 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * i.MX8 NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > +#include <drm/drm_probe_helper.h>
> > +#include <linux/clk-provider.h>
>
> This doesn't seem to be needed.
Dropped.
>
> > +#include <linux/clk.h>
> > +#include <linux/component.h>
>
> Same here.
Dropped (it was a component driver before).
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/irq.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/mfd/syscon/imx8mq-iomuxc-gpr.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/regmap.h>
> > +#include <linux/sys_soc.h>
> > +#include <video/videomode.h>
> > +
> > +#include "nwl-drv.h"
> > +#include "nwl-dsi.h"
> > +
> > +#define DRV_NAME "imx-nwl-dsi"
> > +
> > +/* 8MQ SRC specific registers */
> > +#define SRC_MIPIPHY_RCR 0x28
> > +#define RESET_BYTE_N BIT(1)
> > +#define RESET_N BIT(2)
> > +#define DPI_RESET_N BIT(3)
> > +#define ESC_RESET_N BIT(4)
> > +#define PCLK_RESET_N BIT(5)
> > +
> > +/* Possible clocks */
> > +#define CLK_PIXEL "pixel"
> > +#define CLK_CORE "core"
> > +#define CLK_BYPASS "bypass"
> > +
> > +enum imx_ext_regs {
> > + IMX_REG_CSR = BIT(1),
> > + IMX_REG_SRC = BIT(2),
> > + IMX_REG_GPR = BIT(3),
> > +};
> > +
> > +static const struct regmap_config nwl_dsi_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = IRQ_MASK2,
> > + .name = DRV_NAME,
> > +};
> > +
> > +struct imx_nwl_platform_data {
> > + int (*poweron)(struct imx_nwl_dsi *dsi);
> > + int (*poweroff)(struct imx_nwl_dsi *dsi);
> > + u32 ext_regs; /* required external registers */
> > + struct imx_nwl_clk_config clk_config[NWL_MAX_PLATFORM_CLOCKS];
> > +};
> > +
> > +static inline struct imx_nwl_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> > +{
> > + return container_of(bridge, struct imx_nwl_dsi, bridge);
> > +}
> > +
> > +static void imx_nwl_dsi_set_clocks(struct imx_nwl_dsi *dsi, bool enable)
> > +{
> > + struct device *dev = dsi->dev;
> > + const char *id;
> > + struct clk *clk;
> > + unsigned long new_rate, cur_rate;
> > + bool enabled;
> > + size_t i;
> > + int ret;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "%sabling platform clocks",
> > + enable ? "en" : "dis");
> > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
> > + if (!dsi->clk_config[i].present)
> > + continue;
> > + id = dsi->clk_config[i].id;
> > + clk = dsi->clk_config[i].clk;
> > + new_rate = dsi->clk_config[i].rate;
> > + cur_rate = clk_get_rate(clk);
> > + enabled = dsi->clk_config[i].enabled;
> > +
> > + /* BYPASS clk must have the same rate as PHY_REF clk */
> > + if (!strcmp(id, CLK_BYPASS))
> > + new_rate = clk_get_rate(dsi->phy_ref_clk);
> > +
> > + if (enable) {
> > + if (enabled && new_rate != cur_rate)
> > + clk_disable_unprepare(clk);
> > + else if (enabled && new_rate == cur_rate)
> > + continue;
> > + if (new_rate > 0)
> > + clk_set_rate(clk, new_rate);
> > + ret = clk_prepare_enable(clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to enable clock %s",
> > + id);
> > + }
> > + dsi->clk_config[i].enabled = true;
> > + cur_rate = clk_get_rate(clk);
> > + DRM_DEV_DEBUG_DRIVER(
> > + dev, "Enabled %s clk (rate: req=%lu act=%lu)\n",
> > + id, new_rate, cur_rate);
> > + } else if (enabled) {
> > + clk_disable_unprepare(clk);
> > + dsi->clk_config[i].enabled = false;
> > + DRM_DEV_DEBUG_DRIVER(dev, "Disabled %s clk\n", id);
> > + }
> > + }
> > +}
> > +
> > +static void imx_nwl_dsi_enable(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + int ret;
> > +
> > + imx_nwl_dsi_set_clocks(dsi, true);
> > +
> > + ret = dsi->pdata->poweron(dsi);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dev, "Failed to power on DSI (%d)\n", ret);
> > +}
> > +
> > +static void imx_nwl_dsi_disable(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > +
> > + if (dsi->dpms_mode != DRM_MODE_DPMS_ON)
> > + return;
> > +
>
> The DRM core should guarantee that the bridge won't be disabled twice,
> so I don't think you need this check. Similarly I think the enabled flag
> in the imx_nwl_clk_config structure can be removed.
Dropped and i also simplified the imx_nwl_clk_config - rates are supplied
via DT here anyway.
>
> > + DRM_DEV_DEBUG_DRIVER(dev, "Disabling encoder");
>
> Is this really needed ?
Dropped.
> > + dsi->pdata->poweroff(dsi);
> > + imx_nwl_dsi_set_clocks(dsi, false);
> > +}
> > +
> > +static void imx_nwl_select_input_source(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device_node *remote;
> > + u32 mux_val = IMX8MQ_GPR13_MIPI_MUX_SEL;
> > +
> > + remote = of_graph_get_remote_node(dsi->dev->of_node, 0, 0);
> > + if (strcmp(remote->name, "lcdif") == 0)
> > + mux_val = 0;
> > +
Getting it from dt spares us carrying around more platform specific
state. Should i change it anyway?
>
> Can't you check the remote node at probe time instead of every time the
> bridge gets enabled, and program the IO mux accordingly there ?
>
> This code is i.MX-specific, so it should be isolated in an operation in
> struct imx_nwl_platform_data.
Done.
>
> > + DRM_DEV_INFO(dsi->dev, "Using %s as input source\n",
> > + (mux_val) ? "DCSS" : "LCDIF");
> > + regmap_update_bits(dsi->mux_sel, IOMUXC_GPR13,
> > + IMX8MQ_GPR13_MIPI_MUX_SEL, mux_val);
> > + of_node_put(remote);
> > +}
> > +
> > +static void imx_nwl_dsi_bridge_disable(struct drm_bridge *bridge)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge_to_dsi(bridge);
> > +
> > + if (dsi->dpms_mode != DRM_MODE_DPMS_ON)
> > + return;
> > +
> > + nwl_dsi_disable(dsi);
> > + imx_nwl_dsi_disable(dsi);
> > + pm_runtime_put_sync(dsi->dev);
>
> Do you need a put_sync, wouldn't a put do ?
Switched to put() only.
>
> > + dsi->dpms_mode = DRM_MODE_DPMS_OFF;
> > +}
> > +
> > +static bool
> > +imx_nwl_dsi_bridge_mode_fixup(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode,
> > + struct drm_display_mode *adjusted_mode)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge_to_dsi(bridge);
> > + struct device *dev = dsi->dev;
> > + union phy_configure_opts new_cfg;
> > + unsigned long phy_ref_rate;
> > + int ret;
> > +
> > + ret = nwl_dsi_get_dphy_params(dsi, adjusted_mode, &new_cfg);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /*
> > + * If hs clock is unchanged, we're all good - all parameters are
> > + * derived from it atm.
> > + */
> > + if (new_cfg.mipi_dphy.hs_clk_rate == dsi->phy_cfg.mipi_dphy.hs_clk_rate)
> > + return true;
> > +
> > + phy_ref_rate = clk_get_rate(dsi->phy_ref_clk);
> > + DRM_DEV_DEBUG_DRIVER(dev, "PHY at ref rate: %lu\n", phy_ref_rate);
> > + if (ret < 0) {
>
> This can't happen. Or are you missing a function call before the check
> ?
The code used that a long time ago, fixed that path.
>
> > + DRM_DEV_ERROR(dsi->dev,
> > + "Cannot setup PHY for mode: %ux%u @%d Hz\n",
> > + adjusted_mode->hdisplay, adjusted_mode->vdisplay,
> > + adjusted_mode->clock);
> > + DRM_DEV_ERROR(dsi->dev, "PHY ref clk: %lu, bit clk: %lu\n",
> > + phy_ref_rate, new_cfg.mipi_dphy.hs_clk_rate);
> > + } else {
> > + /* Save the new desired phy config */
> > + memcpy(&dsi->phy_cfg, &new_cfg, sizeof(new_cfg));
>
> The mode_fixup operation shall not change the device state, it can be
> called multiple times when trying modes.
Moved the parts to 'mode_set' instead.
>
> > + }
> > +
> > + /* LCDIF + NWL needs active high sync */
> > + adjusted_mode->flags |= (DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> > + adjusted_mode->flags &= ~(DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC);
> > +
> > + drm_display_mode_to_videomode(adjusted_mode, &dsi->vm);
> > + drm_mode_debug_printmodeline(adjusted_mode);
> > +
> > + return ret == 0;
>
> return 0;
mode_fixup wants a bool but that code is gone anyways.
>
> > +}
> > +
> > +static enum drm_mode_status
> > +imx_nwl_dsi_bridge_mode_valid(struct drm_bridge *bridge,
> > + const struct drm_display_mode *mode)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge_to_dsi(bridge);
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + if (bpp < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Invalid pixel format: %d\n",
> > + dsi->format);
> > + return MODE_BAD;
> > + }
>
> The format isn't part of the mode, so this doesn't belong here. You
> should here instead check that the mode clock and other timing data
> (especially the visible resolution) are within the range supported by
> the device.
O.k. I've added a clock check derived from the spec instead.
> > +
> > + return MODE_OK;
> > +}
> > +
> > +static void imx_nwl_dsi_bridge_pre_enable(struct drm_bridge *bridge)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge_to_dsi(bridge);
> > +
> > + if (dsi->dpms_mode == DRM_MODE_DPMS_ON)
> > + return;
> > +
> > + imx_nwl_select_input_source(dsi);
> > + pm_runtime_get_sync(dsi->dev);
> > + imx_nwl_dsi_enable(dsi);
> > + nwl_dsi_enable(dsi);
> > + dsi->dpms_mode = DRM_MODE_DPMS_ON;
> > +}
> > +
> > +static int imx_nwl_dsi_bridge_attach(struct drm_bridge *bridge)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge->driver_private;
> > + struct drm_encoder *encoder = bridge->encoder;
> > +
> > + if (!encoder) {
> > + DRM_DEV_ERROR(dsi->dev, "Parent encoder object not found\n");
> > + return -ENODEV;
> > + }
>
> Can't this happen ?
Dropped.
>
> > +
> > + /* Set the encoder type as caller does not know it */
> > + bridge->encoder->encoder_type = DRM_MODE_ENCODER_DSI;
>
> The encoder type is quite meaningless and userspace should not depend on
> it, so I wouldn't set it here, especially that the encoder may not
> expect the bridge to override its type.
Dropped.
>
> > +
> > + /* Attach the panel-bridge to the dsi bridge */
> > + return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge);
> > +}
> > +
> > +static void imx_nwl_dsi_bridge_detach(struct drm_bridge *bridge)
> > +{
> > + struct imx_nwl_dsi *dsi = bridge->driver_private;
> > +
> > + drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
>
> This is already done in nwl_dsi_host_detach().
Dropped the whole detach.
>
> > +}
> > +
> > +/* see dw-mipi-dsi.c */
>
> What for ? :-)
Dropped.
>
> > +static const struct drm_bridge_funcs imx_nwl_dsi_bridge_funcs = {
> > + .pre_enable = imx_nwl_dsi_bridge_pre_enable,
> > + .disable = imx_nwl_dsi_bridge_disable,
> > + .mode_fixup = imx_nwl_dsi_bridge_mode_fixup,
> > + .mode_valid = imx_nwl_dsi_bridge_mode_valid,
> > + .attach = imx_nwl_dsi_bridge_attach,
> > + .detach = imx_nwl_dsi_bridge_detach,
> > +};
> > +
> > +static int imx_nwl_dsi_parse_dt(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device_node *np = dsi->dev->of_node;
> > + struct platform_device *pdev = to_platform_device(dsi->dev);
> > + struct resource *res;
> > + struct clk *clk;
> > + const char *clk_id;
> > + void __iomem *base;
> > + int i, ret;
> > +
> > + dsi->phy = devm_phy_get(dsi->dev, "dphy");
> > + if (IS_ERR(dsi->phy)) {
> > + ret = PTR_ERR(dsi->phy);
> > + dev_err(dsi->dev, "Could not get PHY (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + /* Platform dependent clocks */
> > + memcpy(dsi->clk_config, dsi->pdata->clk_config,
> > + sizeof(dsi->pdata->clk_config));
> > +
> > + for (i = 0; i < ARRAY_SIZE(dsi->pdata->clk_config); i++) {
> > + if (!dsi->clk_config[i].present)
> > + continue;
> > +
> > + clk_id = dsi->clk_config[i].id;
> > + clk = devm_clk_get(dsi->dev, clk_id);
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dsi->dev, "Failed to get %s clock (%d)\n",
> > + clk_id, ret);
> > + return ret;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "Setup clk %s (rate: %lu)\n",
> > + clk_id, clk_get_rate(clk));
> > + dsi->clk_config[i].clk = clk;
> > + }
> > +
> > + /* DSI clocks */
> > + clk = devm_clk_get(dsi->dev, "phy_ref");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dsi->dev, "Failed to get phy_ref clock: %d\n", ret);
> > + return ret;
> > + }
> > + dsi->phy_ref_clk = clk;
> > +
> > + clk = devm_clk_get(dsi->dev, "rx_esc");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dsi->dev, "Failed to get rx_esc clock: %d\n", ret);
> > + return ret;
> > + }
> > + dsi->rx_esc_clk = clk;
> > +
> > + clk = devm_clk_get(dsi->dev, "tx_esc");
> > + if (IS_ERR(clk)) {
> > + ret = PTR_ERR(clk);
> > + dev_err(dsi->dev, "Failed to get tx_esc clock: %d\n", ret);
> > + return ret;
> > + }
> > + dsi->tx_esc_clk = clk;
> > +
> > + dsi->csr = syscon_regmap_lookup_by_phandle(np, "csr");
> > + if (IS_ERR(dsi->csr) && dsi->pdata->ext_regs & IMX_REG_CSR) {
> > + ret = PTR_ERR(dsi->csr);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get CSR regmap: %d\n",
> > + ret);
> > + return ret;
> > + }
>
> This doesn't seem to be used anywhere.
That's true. i had it in for the imx8q*, dropped that (and some other
parts) until support for that soc is added.
>
> > + dsi->reset = syscon_regmap_lookup_by_phandle(np, "src");
> > + if (IS_ERR(dsi->reset) && (dsi->pdata->ext_regs & IMX_REG_SRC)) {
> > + ret = PTR_ERR(dsi->reset);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get SRC regmap: %d\n",
> > + ret);
> > + return ret;
> > + }
>
> Couldn't you model a reset controller in that syscon, and use the reset
> controller API here ? It would allow moving the i.MX-specific power on
> and off functions from this driver, making it more generic.
In fact the reset controller is already there, wired it up accordingly.
>
> > + dsi->mux_sel = syscon_regmap_lookup_by_phandle(np, "mux-sel");
> > + if (IS_ERR(dsi->mux_sel) && (dsi->pdata->ext_regs & IMX_REG_GPR)) {
> > + ret = PTR_ERR(dsi->mux_sel);
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get GPR regmap: %d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(dsi->dev, res);
>
> You can replace those two calls with devm_platform_ioremap_resource().
Done.
>
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + dsi->regmap =
> > + devm_regmap_init_mmio(dsi->dev, base, &nwl_dsi_regmap_config);
> > + if (IS_ERR(dsi->regmap)) {
> > + ret = PTR_ERR(dsi->regmap);
> > + DRM_DEV_ERROR(dsi->dev,
> > + "Failed to create NWL DSI regmap: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + dsi->irq = platform_get_irq(pdev, 0);
> > + if (dsi->irq < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Failed to get device IRQ: %d\n",
> > + dsi->irq);
> > + return dsi->irq;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx8mq_dsi_poweron(struct imx_nwl_dsi *dsi)
> > +{
> > + /* otherwise the display stays blank */
> > + usleep_range(200, 300);
> > +
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, PCLK_RESET_N,
> > + PCLK_RESET_N);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, ESC_RESET_N,
> > + ESC_RESET_N);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, RESET_BYTE_N,
> > + RESET_BYTE_N);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, DPI_RESET_N,
> > + DPI_RESET_N);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx8mq_dsi_poweroff(struct imx_nwl_dsi *dsi)
> > +{
> > + if (USE_SRC_RESET_QUIRK(dsi->quirks))
> > + return 0;
> > +
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, PCLK_RESET_N, 0);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, ESC_RESET_N, 0);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, RESET_BYTE_N, 0);
> > + regmap_update_bits(dsi->reset, SRC_MIPIPHY_RCR, DPI_RESET_N, 0);
> > + return 0;
> > +}
> > +
> > +static const struct drm_bridge_timings imx_nwl_timings = {
> > + .input_bus_flags = DRM_BUS_FLAG_DE_LOW,
> > +};
> > +
> > +static struct imx_nwl_platform_data imx8mq_dev = {
>
> This structure should be const, especially as it contains function
> pointers.
Done.
>
> > + .poweron = &imx8mq_dsi_poweron,
> > + .poweroff = &imx8mq_dsi_poweroff,
> > + .clk_config = {
> > + { .id = CLK_CORE, .present = true },
> > + { .id = CLK_PIXEL, .present = false },
> > + { .id = CLK_BYPASS, .present = false },
> > + },
> > + .ext_regs = IMX_REG_SRC | IMX_REG_GPR,
> > +};
> > +
> > +static const struct of_device_id imx_nwl_dsi_dt_ids[] = {
> > + { .compatible = "fsl,imx8mq-nwl-dsi", .data = &imx8mq_dev, },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx_nwl_dsi_dt_ids);
> > +
> > +static const struct soc_device_attribute imx_nwl_quirks_match[] = {
> > + { .soc_id = "i.MX8MQ", .revision = "2.0",
> > + .data = (void *)(E11418_HS_MODE_QUIRK | SRC_RESET_QUIRK) },
> > + { /* sentinel. */ },
> > +};
> > +
> > +static int imx_nwl_dsi_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + const struct of_device_id *of_id =
> > + of_match_device(imx_nwl_dsi_dt_ids, dev);
> > + const struct imx_nwl_platform_data *pdata = of_id->data;
> > + const struct soc_device_attribute *attr;
> > + struct imx_nwl_dsi *dsi;
> > + int ret;
> > +
> > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
> > + if (!dsi)
> > + return -ENOMEM;
> > +
> > + dsi->dev = dev;
> > + dsi->pdata = pdata;
> > + dsi->dpms_mode = DRM_MODE_DPMS_OFF;
>
> DPMS is legacy, let's not use it within the driver.
Removed that since the drm layer keeps the state for us.
> > +
> > + ret = imx_nwl_dsi_parse_dt(dsi);
> > + if (ret)
> > + return ret;
> > +
> > + ret = devm_request_irq(dev, dsi->irq, nwl_dsi_irq_handler, 0,
> > + dev_name(dev), dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to request IRQ: %d (%d)\n", dsi->irq,
> > + ret);
> > + return ret;
> > + }
> > +
> > + dsi->dsi_host.ops = &nwl_dsi_host_ops;
> > + dsi->dsi_host.dev = dev;
> > + ret = mipi_dsi_host_register(&dsi->dsi_host);
> > + if (ret) {
> > + DRM_DEV_ERROR(dev, "Failed to register MIPI host: %d\n", ret);
> > + goto err_cleanup;
> > + }
> > +
> > + attr = soc_device_match(imx_nwl_quirks_match);
> > + if (attr)
> > + dsi->quirks = (uintptr_t)attr->data;
> > +
> > + dsi->bridge.driver_private = dsi;
> > + dsi->bridge.funcs = &imx_nwl_dsi_bridge_funcs;
> > + dsi->bridge.of_node = dev->of_node;
> > + dsi->bridge.timings = &imx_nwl_timings;
> > +
> > + drm_bridge_add(&dsi->bridge);
> > +
> > + dev_set_drvdata(dev, dsi);
> > + pm_runtime_enable(dev);
> > + return 0;
> > +
> > +err_cleanup:
> > + devm_free_irq(dev, dsi->irq, dsi);
> > + return ret;
> > +}
> > +
> > +static int imx_nwl_dsi_remove(struct platform_device *pdev)
> > +{
> > + pm_runtime_disable(&pdev->dev);
>
> You should call drm_bridge_remove() here, not in
> nwl_dsi_host_detach().
I opted to call `mipi_dsi_host_unregister(&dsi->dsi_host)` which would
keep the removal in `nwl_dsi_host_detach()` but would also make sure
it's called on removal (modelled like cdns-dsi).
>
> > + return 0;
> > +}
> > +
> > +static struct platform_driver imx_nwl_dsi_driver = {
> > + .probe = imx_nwl_dsi_probe,
> > + .remove = imx_nwl_dsi_remove,
> > + .driver = {
> > + .of_match_table = imx_nwl_dsi_dt_ids,
> > + .name = DRV_NAME,
> > + },
> > +};
> > +
> > +module_platform_driver(imx_nwl_dsi_driver);
> > +
> > +MODULE_AUTHOR("NXP Semiconductor");
> > +MODULE_AUTHOR("Purism SPC");
> > +MODULE_DESCRIPTION("i.MX8 Northwest Logic MIPI-DSI driver");
> > +MODULE_LICENSE("GPL"); /* GPLv2 or later */
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.h b/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.h
> > new file mode 100644
> > index 000000000000..a1e30c58b627
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/nwl-drv.h
> > @@ -0,0 +1,72 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * i.MX8 NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#ifndef __NWL_DRV_H__
> > +#define __NWL_DRV_H__
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +#include <linux/phy/phy.h>
> > +
> > +struct imx_nwl_platform_data;
> > +
> > +/* i.MX8 NWL quirks */
> > +/* i.MX8MQ errata E11418 */
> > +#define E11418_HS_MODE_QUIRK BIT(0)
> > +#define USE_E11418_HS_MODE_QUIRK(x) ((x) & E11418_HS_MODE_QUIRK)
> > +
> > +/* Skip DSI bits in SRC on disable to avoid blank display on enable */
> > +#define SRC_RESET_QUIRK BIT(1)
> > +#define USE_SRC_RESET_QUIRK(x) ((x) & SRC_RESET_QUIRK)
>
> The USE_* macros are not shorter to type, so I would type out the &
> check explicitly.
Dropped.
> > +
> > +#define NWL_MAX_PLATFORM_CLOCKS 3
> > +struct imx_nwl_clk_config {
> > + const char *id;
> > + struct clk *clk;
> > + bool present;
> > + bool enabled;
> > + u32 rate;
> > +};
> > +
> > +struct imx_nwl_dsi {
> > + struct drm_bridge bridge;
> > + struct mipi_dsi_host dsi_host;
> > + struct drm_bridge *panel_bridge;
> > + struct device *dev;
> > + struct phy *phy;
> > + union phy_configure_opts phy_cfg;
> > + unsigned int quirks;
> > +
> > + struct regmap *regmap;
> > + int irq;
> > +
> > + /* External registers */
> > + struct regmap *csr;
> > + struct regmap *mux_sel;
> > + struct regmap *reset;
> > +
> > + /* Platform dependent clocks */
> > + struct imx_nwl_clk_config clk_config[3];
>
> I would use NWL_MAX_PLATFORM_CLOCKS instead of 3 as
> imx_nwl_platform_data uses the macro.
That was an omission, fixed.
>
> > + /* DSI clocks */
> > + struct clk *phy_ref_clk;
> > + struct clk *rx_esc_clk;
> > + struct clk *tx_esc_clk;
> > +
> > + /* dsi lanes */
> > + u32 lanes;
> > + enum mipi_dsi_pixel_format format;
> > + struct videomode vm;
> > + unsigned long dsi_mode_flags;
> > +
> > + int dpms_mode;
> > +
> > + struct mipi_dsi_transfer *xfer;
> > +
> > + const struct imx_nwl_platform_data *pdata;
> > +};
> > +
> > +#endif /* __NWL_DRV_H__ */
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.c b/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.c
> > new file mode 100644
> > index 000000000000..0e1463af162f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.c
> > @@ -0,0 +1,745 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * NWL DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +
> > +#include <asm/unaligned.h>
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_crtc_helper.h>
> > +#include <drm/drm_of.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_print.h>
> > +#include <linux/clk.h>
> > +#include <linux/irq.h>
> > +#include <linux/regmap.h>
> > +#include <video/mipi_display.h>
> > +#include <video/videomode.h>
> > +
> > +#include "nwl-drv.h"
> > +#include "nwl-dsi.h"
> > +
> > +#define MIPI_FIFO_TIMEOUT msecs_to_jiffies(500)
> > +
> > +/* PKT reg bit manipulation */
> > +#define REG_MASK(e, s) (((1 << ((e) - (s) + 1)) - 1) << (s))
> > +#define REG_PUT(x, e, s) (((x) << (s)) & REG_MASK(e, s))
> > +#define REG_GET(x, e, s) (((x) & REG_MASK(e, s)) >> (s))
>
> Let's not reinvent the wheel, linux/bits.h and linux/bitfield.h can be
> used instead.
Done.
>
> > +
> > +/*
> > + * PKT_CONTROL format:
> > + * [15: 0] - word count
> > + * [17:16] - virtual channel
> > + * [23:18] - data type
> > + * [24] - LP or HS select (0 - LP, 1 - HS)
> > + * [25] - perform BTA after packet is sent
> > + * [26] - perform BTA only, no packet tx
> > + */
> > +#define WC(x) REG_PUT((x), 15, 0)
> > +#define TX_VC(x) REG_PUT((x), 17, 16)
> > +#define TX_DT(x) REG_PUT((x), 23, 18)
> > +#define HS_SEL(x) REG_PUT((x), 24, 24)
> > +#define BTA_TX(x) REG_PUT((x), 25, 25)
> > +#define BTA_NO_TX(x) REG_PUT((x), 26, 26)
> > +
> > +/*
> > + * RX_PKT_HEADER format:
> > + * [15: 0] - word count
> > + * [21:16] - data type
> > + * [23:22] - virtual channel
> > + */
> > +#define RX_DT(x) REG_GET((x), 21, 16)
> > +#define RX_VC(x) REG_GET((x), 23, 22)
> > +
> > +/*
> > + * DSI Video mode
> > + */
> > +#define VIDEO_MODE_BURST_MODE_WITH_SYNC_PULSES 0
> > +#define VIDEO_MODE_NON_BURST_MODE_WITH_SYNC_EVENTS BIT(0)
> > +#define VIDEO_MODE_BURST_MODE BIT(1)
> > +
> > +/*
> > + * DPI color coding
> > + */
> > +#define DPI_16_BIT_565_PACKED 0
> > +#define DPI_16_BIT_565_ALIGNED 1
> > +#define DPI_16_BIT_565_SHIFTED 2
> > +#define DPI_18_BIT_PACKED 3
> > +#define DPI_18_BIT_ALIGNED 4
> > +#define DPI_24_BIT 5
> > +
> > +/*
> > + * DPI Pixel format
> > + */
> > +#define PIXEL_FORMAT_16 0
> > +#define PIXEL_FORMAT_18 BIT(0)
> > +#define PIXEL_FORMAT_18L BIT(1)
> > +#define PIXEL_FORMAT_24 (BIT(0) | BIT(1))
> > +
> > +enum transfer_direction { DSI_PACKET_SEND, DSI_PACKET_RECEIVE };
>
> Line breaks please.
Done.
>
> > +
> > +struct mipi_dsi_transfer {
>
> Let's not use such a generic name for a driver-specific structure. You
> should name is nwl_dsi_transfer.
Fixed.
>
> > + const struct mipi_dsi_msg *msg;
> > + struct mipi_dsi_packet packet;
> > + struct completion completed;
> > +
> > + int status; /* status of transmission */
> > + enum transfer_direction direction;
> > + bool need_bta;
> > + u8 cmd;
> > + u16 rx_word_count;
> > + size_t tx_len; /* bytes sent */
> > + size_t rx_len; /* bytes received */
> > +};
> > +
> > +static inline int nwl_dsi_write(struct imx_nwl_dsi *dsi, unsigned int reg,
> > + u32 val)
> > +{
> > + int ret;
> > +
> > + ret = regmap_write(dsi->regmap, reg, val);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev,
> > + "Failed to write NWL DSI reg 0x%x: %d\n", reg,
> > + ret);
> > + return ret;
> > +}
> > +
> > +static inline u32 nwl_dsi_read(struct imx_nwl_dsi *dsi, u32 reg)
> > +{
> > + unsigned int val;
> > + int ret;
> > +
> > + ret = regmap_read(dsi->regmap, reg, &val);
> > + if (ret < 0)
> > + DRM_DEV_ERROR(dsi->dev, "Failed to read NWL DSI reg 0x%x: %d\n",
> > + reg, ret);
> > +
> > + return val;
>
> You're loosing the error...
Looking at other drivers they often just ignore the return value of
regmap_read. The places we use this can't do much to recover from errors
so i figured it'd be best to at least log it so it becomes debuggable
instead of just dropping it to the floor.
> > +}
> > +
> > +static u32 nwl_dsi_get_dpi_pixel_format(enum mipi_dsi_pixel_format format)
> > +{
> > + switch (format) {
> > + case MIPI_DSI_FMT_RGB565:
> > + return PIXEL_FORMAT_16;
> > + case MIPI_DSI_FMT_RGB666:
> > + return PIXEL_FORMAT_18L;
> > + case MIPI_DSI_FMT_RGB666_PACKED:
> > + return PIXEL_FORMAT_18;
> > + case MIPI_DSI_FMT_RGB888:
> > + return PIXEL_FORMAT_24;
> > + default:
> > + return -EINVAL;
> > + }
> > +}
> > +
> > +int nwl_dsi_get_dphy_params(struct imx_nwl_dsi *dsi,
> > + const struct drm_display_mode *mode,
> > + union phy_configure_opts *phy_opts)
> > +{
> > + unsigned long rate;
> > +
> > + if (dsi->lanes < 1 || dsi->lanes > 4)
> > + return -EINVAL;
> > +
> > + /*
> > + * So far the DPHY spec minimal timings work for both mixel
> > + * dphy and nwl dsi host
> > + */
> > + phy_mipi_dphy_get_default_config(
> > + mode->crtc_clock * 1000,
> > + mipi_dsi_pixel_format_to_bpp(dsi->format), dsi->lanes,
> > + &phy_opts->mipi_dphy);
> > + rate = clk_get_rate(dsi->tx_esc_clk);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "LP clk is @%lu Hz\n", rate);
> > + phy_opts->mipi_dphy.lp_clk_rate = rate;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(nwl_dsi_get_dphy_params);
>
> No need to export symbols, those fubctions are only meant to be called
> from within the same module.
Removed (also left overs when i had these in two modules).
>
> > +
> > +#define PSEC_PER_SEC 1000000000000LL
> > +/*
> > + * ps2bc - Picoseconds to byte clock cycles
> > + */
> > +static u32 ps2bc(struct imx_nwl_dsi *dsi, unsigned long long ps)
> > +{
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + return DIV_ROUND_UP(ps * dsi->vm.pixelclock * bpp,
> > + dsi->lanes * 8 * PSEC_PER_SEC);
> > +}
> > +
> > +/**
> > + * ui2bc - UI time periods to byte clock cycles
> > + */
> > +static u32 ui2bc(struct imx_nwl_dsi *dsi, unsigned long long ui)
> > +{
> > + int bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
> > +
> > + return DIV_ROUND_UP(ui * dsi->lanes, dsi->vm.pixelclock * bpp);
> > +}
> > +
> > +#define USEC_PER_SEC 1000000L
> > +/*
> > + * us2bc - micro seconds to lp clock cycles
> > + */
> > +static u32 us2lp(u32 lp_clk_rate, unsigned long us)
> > +{
> > + return DIV_ROUND_UP(us * lp_clk_rate, USEC_PER_SEC);
> > +}
> > +
> > +static int nwl_dsi_config_host(struct imx_nwl_dsi *dsi)
> > +{
> > + u32 cycles;
> > + struct phy_configure_opts_mipi_dphy *cfg = &dsi->phy_cfg.mipi_dphy;
> > +
> > + if (dsi->lanes < 1 || dsi->lanes > 4)
> > + return -EINVAL;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "DSI Lanes %d\n", dsi->lanes);
> > + nwl_dsi_write(dsi, CFG_NUM_LANES, dsi->lanes - 1);
> > +
> > + if (dsi->dsi_mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
> > + nwl_dsi_write(dsi, CFG_NONCONTINUOUS_CLK, 0x01);
> > + nwl_dsi_write(dsi, CFG_AUTOINSERT_EOTP, 0x01);
> > + } else {
> > + nwl_dsi_write(dsi, CFG_NONCONTINUOUS_CLK, 0x00);
> > + nwl_dsi_write(dsi, CFG_AUTOINSERT_EOTP, 0x00);
> > + }
> > +
> > + /* values in byte clock cycles */
> > + cycles = ui2bc(dsi, cfg->clk_pre);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_t_pre: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, CFG_T_PRE, cycles);
> > + cycles = ps2bc(dsi, cfg->lpx + cfg->clk_prepare + cfg->clk_zero);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap (pre): 0x%x\n", cycles);
> > + cycles += ui2bc(dsi, cfg->clk_pre);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, CFG_T_POST, cycles);
> > + cycles = ps2bc(dsi, cfg->hs_exit);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_tx_gap: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, CFG_TX_GAP, cycles);
> > +
> > + nwl_dsi_write(dsi, CFG_EXTRA_CMDS_AFTER_EOTP, 0x01);
> > + nwl_dsi_write(dsi, CFG_HTX_TO_COUNT, 0x00);
> > + nwl_dsi_write(dsi, CFG_LRX_H_TO_COUNT, 0x00);
> > + nwl_dsi_write(dsi, CFG_BTA_H_TO_COUNT, 0x00);
> > + /* In LP clock cycles */
> > + cycles = us2lp(cfg->lp_clk_rate, cfg->wakeup);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "cfg_twakeup: 0x%x\n", cycles);
> > + nwl_dsi_write(dsi, CFG_TWAKEUP, cycles);
> > +
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_config_dpi(struct imx_nwl_dsi *dsi)
> > +{
> > + struct videomode *vm = &dsi->vm;
> > + u32 color_format, mode;
> > + bool burst_mode;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hfront_porch = %d\n", vm->hfront_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hback_porch = %d\n", vm->hback_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hsync_len = %d\n", vm->hsync_len);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "hactive = %d\n", vm->hactive);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vfront_porch = %d\n", vm->vfront_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vback_porch = %d\n", vm->vback_porch);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vsync_len = %d\n", vm->vsync_len);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "vactive = %d\n", vm->vactive);
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "clock = %lu kHz\n",
> > + vm->pixelclock / 1000);
> > +
> > + color_format = nwl_dsi_get_dpi_pixel_format(dsi->format);
> > + if (color_format < 0) {
> > + DRM_DEV_ERROR(dsi->dev, "Invalid color format 0x%x\n",
> > + dsi->format);
> > + return color_format;
> > + }
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev, "pixel fmt = %d\n", dsi->format);
> > +
> > + nwl_dsi_write(dsi, INTERFACE_COLOR_CODING, DPI_24_BIT);
> > + nwl_dsi_write(dsi, PIXEL_FORMAT, color_format);
> > + /*
> > + * Adjusting input polarity based on the video mode results in
> > + * a black screen so always pick active low:
> > + */
> > + nwl_dsi_write(dsi, VSYNC_POLARITY, VSYNC_POLARITY_ACTIVE_LOW);
> > + nwl_dsi_write(dsi, HSYNC_POLARITY, HSYNC_POLARITY_ACTIVE_LOW);
> > +
> > + burst_mode = (dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_BURST) &&
> > + !(dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE);
> > +
> > + if (burst_mode) {
> > + nwl_dsi_write(dsi, VIDEO_MODE, VIDEO_MODE_BURST_MODE);
> > + nwl_dsi_write(dsi, PIXEL_FIFO_SEND_LEVEL, 256);
> > + } else {
> > + mode = ((dsi->dsi_mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) ?
> > + VIDEO_MODE_BURST_MODE_WITH_SYNC_PULSES :
> > + VIDEO_MODE_NON_BURST_MODE_WITH_SYNC_EVENTS);
> > + nwl_dsi_write(dsi, VIDEO_MODE, mode);
> > + nwl_dsi_write(dsi, PIXEL_FIFO_SEND_LEVEL, vm->hactive);
> > + }
> > +
> > + nwl_dsi_write(dsi, HFP, vm->hfront_porch);
> > + nwl_dsi_write(dsi, HBP, vm->hback_porch);
> > + nwl_dsi_write(dsi, HSA, vm->hsync_len);
> > +
> > + nwl_dsi_write(dsi, ENABLE_MULT_PKTS, 0x0);
> > + nwl_dsi_write(dsi, BLLP_MODE, 0x1);
> > + nwl_dsi_write(dsi, ENABLE_MULT_PKTS, 0x0);
> > + nwl_dsi_write(dsi, USE_NULL_PKT_BLLP, 0x0);
> > + nwl_dsi_write(dsi, VC, 0x0);
> > +
> > + nwl_dsi_write(dsi, PIXEL_PAYLOAD_SIZE, vm->hactive);
> > + nwl_dsi_write(dsi, VACTIVE, vm->vactive - 1);
> > + nwl_dsi_write(dsi, VBP, vm->vback_porch);
> > + nwl_dsi_write(dsi, VFP, vm->vfront_porch);
> > +
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_enable_tx_clock(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(dsi->tx_esc_clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to enable tx_esc clk: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "Enabled tx_esc clk @%lu Hz\n",
> > + clk_get_rate(dsi->tx_esc_clk));
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_enable_rx_clock(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + int ret;
> > +
> > + ret = clk_prepare_enable(dsi->rx_esc_clk);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to enable rx_esc clk: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "Enabled rx_esc clk @%lu Hz\n",
> > + clk_get_rate(dsi->rx_esc_clk));
> > + return 0;
> > +}
> > +
> > +static void nwl_dsi_init_interrupts(struct imx_nwl_dsi *dsi)
> > +{
> > + u32 irq_enable;
> > +
> > + nwl_dsi_write(dsi, IRQ_MASK, 0xffffffff);
> > + nwl_dsi_write(dsi, IRQ_MASK2, 0x7);
> > +
> > + irq_enable = ~(u32)(TX_PKT_DONE_MASK | RX_PKT_HDR_RCVD_MASK |
> > + TX_FIFO_OVFLW_MASK | HS_TX_TIMEOUT_MASK);
> > +
> > + nwl_dsi_write(dsi, IRQ_MASK, irq_enable);
> > +}
> > +
> > +static int nwl_dsi_host_attach(struct mipi_dsi_host *dsi_host,
> > + struct mipi_dsi_device *device)
> > +{
> > + struct imx_nwl_dsi *dsi =
> > + container_of(dsi_host, struct imx_nwl_dsi, dsi_host);
> > + struct device *dev = dsi->dev;
> > + struct drm_bridge *bridge;
> > + struct drm_panel *panel;
> > + int ret;
> > +
> > + DRM_DEV_INFO(dev, "lanes=%u, format=0x%x flags=0x%lx\n", device->lanes,
> > + device->format, device->mode_flags);
> > +
> > + if (device->lanes < 1 || device->lanes > 4)
> > + return -EINVAL;
> > +
> > + dsi->lanes = device->lanes;
> > + dsi->format = device->format;
> > + dsi->dsi_mode_flags = device->mode_flags;
> > +
> > + ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> > + &bridge);
> > + if (ret)
> > + return ret;
> > +
> > + if (panel) {
> > + bridge = drm_panel_bridge_add(panel, DRM_MODE_CONNECTOR_DSI);
> > + if (IS_ERR(bridge))
> > + return PTR_ERR(bridge);
> > + }
> > +
> > + dsi->panel_bridge = bridge;
> > + drm_bridge_add(&dsi->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static int nwl_dsi_host_detach(struct mipi_dsi_host *dsi_host,
> > + struct mipi_dsi_device *device)
> > +{
> > + struct imx_nwl_dsi *dsi =
> > + container_of(dsi_host, struct imx_nwl_dsi, dsi_host);
> > +
> > + drm_of_panel_bridge_remove(dsi->dev->of_node, 1, 0);
> > + drm_bridge_remove(&dsi->bridge);
> > +
> > + return 0;
> > +}
> > +
> > +static bool nwl_dsi_read_packet(struct imx_nwl_dsi *dsi, u32 status)
> > +{
> > + struct device *dev = dsi->dev;
> > + struct mipi_dsi_transfer *xfer = dsi->xfer;
> > + u8 *payload = xfer->msg->rx_buf;
> > + u32 val;
> > + u16 word_count;
> > + u8 channel;
> > + u8 data_type;
> > +
> > + xfer->status = 0;
> > +
> > + if (xfer->rx_word_count == 0) {
> > + if (!(status & RX_PKT_HDR_RCVD))
> > + return false;
> > + /* Get the RX header and parse it */
> > + val = nwl_dsi_read(dsi, RX_PKT_HEADER);
> > + word_count = WC(val);
> > + channel = RX_VC(val);
> > + data_type = RX_DT(val);
> > +
> > + if (channel != xfer->msg->channel) {
> > + DRM_DEV_ERROR(dev,
> > + "[%02X] Channel mismatch (%u != %u)\n",
> > + xfer->cmd, channel, xfer->msg->channel);
> > + return true;
> > + }
> > +
> > + switch (data_type) {
> > + case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_2BYTE:
> > + /* Fall through */
> > + case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
> > + if (xfer->msg->rx_len > 1) {
> > + /* read second byte */
> > + payload[1] = word_count >> 8;
> > + ++xfer->rx_len;
> > + }
> > + /* Fall through */
> > + case MIPI_DSI_RX_GENERIC_SHORT_READ_RESPONSE_1BYTE:
> > + /* Fall through */
> > + case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
> > + if (xfer->msg->rx_len > 0) {
> > + /* read first byte */
> > + payload[0] = word_count & 0xff;
> > + ++xfer->rx_len;
> > + }
> > + xfer->status = xfer->rx_len;
> > + return true;
> > + case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
> > + word_count &= 0xff;
> > + DRM_DEV_ERROR(dev, "[%02X] DSI error report: 0x%02x\n",
> > + xfer->cmd, word_count);
> > + xfer->status = -EPROTO;
> > + return true;
> > + }
> > +
> > + if (word_count > xfer->msg->rx_len) {
> > + DRM_DEV_ERROR(
> > + dev,
> > + "[%02X] Receive buffer too small: %lu (< %u)\n",
> > + xfer->cmd, xfer->msg->rx_len, word_count);
> > + return true;
> > + }
> > +
> > + xfer->rx_word_count = word_count;
> > + } else {
> > + /* Set word_count from previous header read */
> > + word_count = xfer->rx_word_count;
> > + }
> > +
> > + /* If RX payload is not yet received, wait for it */
> > + if (!(status & RX_PKT_PAYLOAD_DATA_RCVD))
> > + return false;
> > +
> > + /* Read the RX payload */
> > + while (word_count >= 4) {
> > + val = nwl_dsi_read(dsi, RX_PAYLOAD);
> > + payload[0] = (val >> 0) & 0xff;
> > + payload[1] = (val >> 8) & 0xff;
> > + payload[2] = (val >> 16) & 0xff;
> > + payload[3] = (val >> 24) & 0xff;
> > + payload += 4;
> > + xfer->rx_len += 4;
> > + word_count -= 4;
> > + }
> > +
> > + if (word_count > 0) {
> > + val = nwl_dsi_read(dsi, RX_PAYLOAD);
> > + switch (word_count) {
> > + case 3:
> > + payload[2] = (val >> 16) & 0xff;
> > + ++xfer->rx_len;
> > + /* Fall through */
> > + case 2:
> > + payload[1] = (val >> 8) & 0xff;
> > + ++xfer->rx_len;
> > + /* Fall through */
> > + case 1:
> > + payload[0] = (val >> 0) & 0xff;
> > + ++xfer->rx_len;
> > + break;
> > + }
> > + }
> > +
> > + xfer->status = xfer->rx_len;
> > +
> > + return true;
> > +}
> > +
> > +static void nwl_dsi_finish_transmission(struct imx_nwl_dsi *dsi, u32 status)
> > +{
> > + struct mipi_dsi_transfer *xfer = dsi->xfer;
> > + bool end_packet = false;
> > +
> > + if (!xfer)
> > + return;
> > +
> > + if (status & TX_FIFO_OVFLW) {
> > + DRM_DEV_ERROR_RATELIMITED(dsi->dev, "tx fifo overflow\n");
> > + return;
> > + }
> > +
> > + if (status & HS_TX_TIMEOUT) {
> > + DRM_DEV_ERROR_RATELIMITED(dsi->dev, "HS tx timeout\n");
> > + return;
> > + }
> > +
> > + if (xfer->direction == DSI_PACKET_SEND && status & TX_PKT_DONE) {
> > + xfer->status = xfer->tx_len;
> > + end_packet = true;
> > + } else if (status & DPHY_DIRECTION &&
> > + ((status & (RX_PKT_HDR_RCVD | RX_PKT_PAYLOAD_DATA_RCVD)))) {
> > + end_packet = nwl_dsi_read_packet(dsi, status);
> > + }
> > +
> > + if (end_packet)
> > + complete(&xfer->completed);
> > +}
> > +
> > +static void nwl_dsi_begin_transmission(struct imx_nwl_dsi *dsi)
> > +{
> > + struct mipi_dsi_transfer *xfer = dsi->xfer;
> > + struct mipi_dsi_packet *pkt = &xfer->packet;
> > + const u8 *payload;
> > + size_t length;
> > + u16 word_count;
> > + u8 hs_mode;
> > + u32 val;
> > + u32 hs_workaround = 0;
> > +
> > + /* Send the payload, if any */
> > + length = pkt->payload_length;
> > + payload = pkt->payload;
> > +
> > + while (length >= 4) {
> > + val = get_unaligned_le32(payload);
>
> The framework doesn't guarantee the payload to be aligned on a multiple
> of 4 bytes ?
It does as far as i can tell, dropped.
>
> > + hs_workaround |= !(val & 0xFFFF00);
> > + nwl_dsi_write(dsi, TX_PAYLOAD, val);
> > + payload += 4;
> > + length -= 4;
> > + }
> > + /* Send the rest of the payload */
> > + val = 0;
> > + switch (length) {
> > + case 3:
> > + val |= payload[2] << 16;
> > + /* Fall through */
> > + case 2:
> > + val |= payload[1] << 8;
> > + hs_workaround |= !(val & 0xFFFF00);
> > + /* Fall through */
> > + case 1:
> > + val |= payload[0];
> > + nwl_dsi_write(dsi, TX_PAYLOAD, val);
> > + break;
> > + }
> > + xfer->tx_len = pkt->payload_length;
> > +
> > + /*
> > + * Send the header
> > + * header[0] = Virtual Channel + Data Type
> > + * header[1] = Word Count LSB (LP) or first param (SP)
> > + * header[2] = Word Count MSB (LP) or second param (SP)
> > + */
> > + word_count = pkt->header[1] | (pkt->header[2] << 8);
> > + if ((hs_workaround && USE_E11418_HS_MODE_QUIRK(dsi->quirks))) {
> > + DRM_DEV_DEBUG_DRIVER(dsi->dev,
> > + "Using hs mode workaround for cmd 0x%x\n",
> > + xfer->cmd);
> > + hs_mode = 1;
> > + } else {
> > + hs_mode = (xfer->msg->flags & MIPI_DSI_MSG_USE_LPM) ? 0 : 1;
> > + }
> > + val = WC(word_count) |
> > + TX_VC(xfer->msg->channel) |
> > + TX_DT(xfer->msg->type) |
> > + HS_SEL(hs_mode) |
> > + BTA_TX(xfer->need_bta);
> > + nwl_dsi_write(dsi, PKT_CONTROL, val);
> > +
> > + /* Send packet command */
> > + nwl_dsi_write(dsi, SEND_PACKET, 0x1);
> > +}
> > +
> > +static ssize_t nwl_dsi_host_transfer(struct mipi_dsi_host *dsi_host,
> > + const struct mipi_dsi_msg *msg)
> > +{
> > + struct imx_nwl_dsi *dsi =
> > + container_of(dsi_host, struct imx_nwl_dsi, dsi_host);
> > + struct mipi_dsi_transfer xfer;
> > + ssize_t ret = 0;
> > +
> > + /* Create packet to be sent */
> > + dsi->xfer = &xfer;
> > + ret = mipi_dsi_create_packet(&xfer.packet, msg);
> > + if (ret < 0) {
> > + dsi->xfer = NULL;
> > + return ret;
> > + }
> > +
> > + if ((msg->type & MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM ||
> > + msg->type & MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM ||
> > + msg->type & MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM ||
> > + msg->type & MIPI_DSI_DCS_READ) &&
> > + msg->rx_len > 0 && msg->rx_buf != NULL)
> > + xfer.direction = DSI_PACKET_RECEIVE;
> > + else
> > + xfer.direction = DSI_PACKET_SEND;
> > +
> > + xfer.need_bta = (xfer.direction == DSI_PACKET_RECEIVE);
> > + xfer.need_bta |= (msg->flags & MIPI_DSI_MSG_REQ_ACK) ? 1 : 0;
> > + xfer.msg = msg;
> > + xfer.status = -ETIMEDOUT;
> > + xfer.rx_word_count = 0;
> > + xfer.rx_len = 0;
> > + xfer.cmd = 0x00;
> > + if (msg->tx_len > 0)
> > + xfer.cmd = ((u8 *)(msg->tx_buf))[0];
> > + init_completion(&xfer.completed);
> > +
> > + nwl_dsi_enable_rx_clock(dsi);
> > +
> > + /* Initiate the DSI packet transmision */
> > + nwl_dsi_begin_transmission(dsi);
> > +
> > + if (!wait_for_completion_timeout(&xfer.completed, MIPI_FIFO_TIMEOUT)) {
> > + DRM_DEV_ERROR(dsi_host->dev, "[%02X] DSI transfer timed out\n",
> > + xfer.cmd);
> > + ret = -ETIMEDOUT;
> > + } else {
> > + ret = xfer.status;
> > + }
> > +
> > + clk_disable_unprepare(dsi->rx_esc_clk);
> > +
> > + return ret;
> > +}
> > +
> > +const struct mipi_dsi_host_ops nwl_dsi_host_ops = {
> > + .attach = nwl_dsi_host_attach,
> > + .detach = nwl_dsi_host_detach,
> > + .transfer = nwl_dsi_host_transfer,
> > +};
> > +
> > +irqreturn_t nwl_dsi_irq_handler(int irq, void *data)
> > +{
> > + u32 irq_status;
> > + struct imx_nwl_dsi *dsi = data;
> > +
> > + irq_status = nwl_dsi_read(dsi, IRQ_STATUS);
> > +
> > + if (irq_status & TX_PKT_DONE || irq_status & RX_PKT_HDR_RCVD ||
> > + irq_status & RX_PKT_PAYLOAD_DATA_RCVD)
> > + nwl_dsi_finish_transmission(dsi, irq_status);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +EXPORT_SYMBOL_GPL(nwl_dsi_irq_handler);
> > +
> > +int nwl_dsi_enable(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > + union phy_configure_opts *phy_cfg = &dsi->phy_cfg;
> > + int ret;
> > +
> > + if (!dsi->lanes) {
> > + DRM_DEV_ERROR(dev, "Need DSI lanes: %d\n", dsi->lanes);
> > + return -EINVAL;
> > + }
> > +
> > + ret = phy_init(dsi->phy);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to init DSI phy: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = phy_configure(dsi->phy, phy_cfg);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to configure DSI phy: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = nwl_dsi_enable_tx_clock(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to enable tx clock: %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = nwl_dsi_config_host(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to set up DSI: %d", ret);
> > + return ret;
> > + }
> > +
> > + ret = nwl_dsi_config_dpi(dsi);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to set up DPI: %d", ret);
> > + return ret;
> > + }
> > +
> > + ret = phy_power_on(dsi->phy);
> > + if (ret < 0) {
> > + DRM_DEV_ERROR(dev, "Failed to power on DPHY (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + nwl_dsi_init_interrupts(dsi);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(nwl_dsi_enable);
> > +
> > +int nwl_dsi_disable(struct imx_nwl_dsi *dsi)
> > +{
> > + struct device *dev = dsi->dev;
> > +
> > + DRM_DEV_DEBUG_DRIVER(dev, "Disabling clocks and phy\n");
> > +
> > + phy_power_off(dsi->phy);
> > + phy_exit(dsi->phy);
> > +
> > + /* Disabling the clock before the phy breaks enabling dsi again */
> > + clk_disable_unprepare(dsi->tx_esc_clk);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(nwl_dsi_disable);
> > diff --git a/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.h b/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.h
> > new file mode 100644
> > index 000000000000..7bcf804843e2
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx-nwl/nwl-dsi.h
> > @@ -0,0 +1,111 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * i.MX8 NWL MIPI DSI host driver
> > + *
> > + * Copyright (C) 2017 NXP
> > + * Copyright (C) 2019 Purism SPC
> > + */
> > +#ifndef __NWL_DSI_H__
> > +#define __NWL_DSI_H__
> > +
> > +#include <drm/drm_mipi_dsi.h>
> > +
> > +/* DSI HOST registers */
> > +#define CFG_NUM_LANES 0x0
>
> Some of the register names are quite prone to namespace clashes. I
> recommend prefixing them all with NWL_DSI_.
Done, also the defines in nwl-dsi.c.
>
> > +#define CFG_NONCONTINUOUS_CLK 0x4
> > +#define CFG_T_PRE 0x8
> > +#define CFG_T_POST 0xc
> > +#define CFG_TX_GAP 0x10
> > +#define CFG_AUTOINSERT_EOTP 0x14
> > +#define CFG_EXTRA_CMDS_AFTER_EOTP 0x18
> > +#define CFG_HTX_TO_COUNT 0x1c
> > +#define CFG_LRX_H_TO_COUNT 0x20
> > +#define CFG_BTA_H_TO_COUNT 0x24
> > +#define CFG_TWAKEUP 0x28
> > +#define CFG_STATUS_OUT 0x2c
> > +#define RX_ERROR_STATUS 0x30
> > +
> > +/* DSI DPI registers */
> > +#define PIXEL_PAYLOAD_SIZE 0x200
> > +#define PIXEL_FIFO_SEND_LEVEL 0x204
> > +#define INTERFACE_COLOR_CODING 0x208
> > +#define PIXEL_FORMAT 0x20c
> > +#define VSYNC_POLARITY 0x210
> > +#define VSYNC_POLARITY_ACTIVE_LOW 0
> > +#define VSYNC_POLARITY_ACTIVE_HIGH BIT(1)
> > +
> > +#define HSYNC_POLARITY 0x214
> > +#define HSYNC_POLARITY_ACTIVE_LOW 0
> > +#define HSYNC_POLARITY_ACTIVE_HIGH BIT(1)
> > +
> > +#define VIDEO_MODE 0x218
> > +#define HFP 0x21c
> > +#define HBP 0x220
> > +#define HSA 0x224
> > +#define ENABLE_MULT_PKTS 0x228
> > +#define VBP 0x22c
> > +#define VFP 0x230
> > +#define BLLP_MODE 0x234
> > +#define USE_NULL_PKT_BLLP 0x238
> > +#define VACTIVE 0x23c
> > +#define VC 0x240
> > +
> > +/* DSI APB PKT control */
> > +#define TX_PAYLOAD 0x280
> > +#define PKT_CONTROL 0x284
> > +#define SEND_PACKET 0x288
> > +#define PKT_STATUS 0x28c
> > +#define PKT_FIFO_WR_LEVEL 0x290
> > +#define PKT_FIFO_RD_LEVEL 0x294
> > +#define RX_PAYLOAD 0x298
> > +#define RX_PKT_HEADER 0x29c
> > +
> > +/* DSI IRQ handling */
> > +#define IRQ_STATUS 0x2a0
> > +#define SM_NOT_IDLE BIT(0)
> > +#define TX_PKT_DONE BIT(1)
> > +#define DPHY_DIRECTION BIT(2)
> > +#define TX_FIFO_OVFLW BIT(3)
> > +#define TX_FIFO_UDFLW BIT(4)
> > +#define RX_FIFO_OVFLW BIT(5)
> > +#define RX_FIFO_UDFLW BIT(6)
> > +#define RX_PKT_HDR_RCVD BIT(7)
> > +#define RX_PKT_PAYLOAD_DATA_RCVD BIT(8)
> > +#define BTA_TIMEOUT BIT(29)
> > +#define LP_RX_TIMEOUT BIT(30)
> > +#define HS_TX_TIMEOUT BIT(31)
> > +
> > +#define IRQ_STATUS2 0x2a4
> > +#define SINGLE_BIT_ECC_ERR BIT(0)
> > +#define MULTI_BIT_ECC_ERR BIT(1)
> > +#define CRC_ERR BIT(2)
> > +
> > +#define IRQ_MASK 0x2a8
> > +#define SM_NOT_IDLE_MASK BIT(0)
> > +#define TX_PKT_DONE_MASK BIT(1)
> > +#define DPHY_DIRECTION_MASK BIT(2)
> > +#define TX_FIFO_OVFLW_MASK BIT(3)
> > +#define TX_FIFO_UDFLW_MASK BIT(4)
> > +#define RX_FIFO_OVFLW_MASK BIT(5)
> > +#define RX_FIFO_UDFLW_MASK BIT(6)
> > +#define RX_PKT_HDR_RCVD_MASK BIT(7)
> > +#define RX_PKT_PAYLOAD_DATA_RCVD_MASK BIT(8)
> > +#define BTA_TIMEOUT_MASK BIT(29)
> > +#define LP_RX_TIMEOUT_MASK BIT(30)
> > +#define HS_TX_TIMEOUT_MASK BIT(31)
> > +
> > +#define IRQ_MASK2 0x2ac
> > +#define SINGLE_BIT_ECC_ERR_MASK BIT(0)
> > +#define MULTI_BIT_ECC_ERR_MASK BIT(1)
> > +#define CRC_ERR_MASK BIT(2)
> > +
> > +extern const struct mipi_dsi_host_ops nwl_dsi_host_ops;
> > +
> > +irqreturn_t nwl_dsi_irq_handler(int irq, void *data);
> > +int nwl_dsi_enable(struct imx_nwl_dsi *dsi);
> > +int nwl_dsi_disable(struct imx_nwl_dsi *dsi);
> > +int nwl_dsi_get_dphy_params(struct imx_nwl_dsi *dsi,
> > + const struct drm_display_mode *mode,
> > + union phy_configure_opts *phy_opts);
> > +
> > +#endif /* __NWL_DSI_H__ */
>
> --
> Regards,
>
> Laurent Pinchart
>
Cheers and thanks for having a look!
-- Guido
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox